Rollup merge of #111279 - compiler-errors:core-item-resolve, r=cjgillot

matthiaskrgr · web-flow · commit 3cb1a4676a62 · 2023-05-06T13:30:06.000+02:00
More robust debug assertions for `Instance::resolve` on built-in traits with non-standard trait items In #111264, a user added a new item to the `Future` trait, but the code in [`resolve_associated_item`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ty_utils/instance/fn.resolve_associated_item.html) implicitly assumes that the `Future` trait is defined with only one method (`Future::poll`) and treats the generator body as the implementation of that method. This PR adds some debug assertions to make sure that that new methods defined on `Future`/`Generator`/etc. don't accidentally resolve to the wrong item when they are added, and adds a helpful comment guiding a compiler dev (or curious `#![no_core]` user) to what must be done to support adding new associated items to these built-in implementations. I am open to discuss whether a test should be added, but I chose against it because I opted to make these `bug!()`s instead of, e.g., diagnostics or fatal errors. Arguably it doesn't need a test because it's not a bug that can be triggered by an end user, and internal-facing misuses of core kind of touch on rust-lang/compiler-team#620 -- however, I think the assertions I added in this PR are still a very useful way to make sure this bug doesn't waste debugging resources down the line. Fixes #111264
diff --git a/compiler/rustc_middle/src/ty/instance.rs b/compiler/rustc_middle/src/ty/instance.rs
@@ -385,7 +385,7 @@ impl<'tcx> Instance<'tcx> {
     /// couldn't complete due to errors elsewhere - this is distinct
     /// from `Ok(None)` to avoid misleading diagnostics when an error
     /// has already been/will be emitted, for the original cause
-    #[instrument(level = "debug", skip(tcx))]
+    #[instrument(level = "debug", skip(tcx), ret)]
     pub fn resolve(
         tcx: TyCtxt<'tcx>,
         param_env: ty::ParamEnv<'tcx>,
diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs
@@ -1207,6 +1207,7 @@ symbols! {
         require,
         residual,
         result,
+        resume,
         return_position_impl_trait_in_trait,
         return_type_notation,
         rhs,
diff --git a/compiler/rustc_ty_utils/src/instance.rs b/compiler/rustc_ty_utils/src/instance.rs
@@ -177,15 +177,55 @@ fn resolve_associated_item<'tcx>(
 
             Some(ty::Instance::new(leaf_def.item.def_id, substs))
         }
-        traits::ImplSource::Generator(generator_data) => Some(Instance {
-            def: ty::InstanceDef::Item(generator_data.generator_def_id),
-            substs: generator_data.substs,
-        }),
-        traits::ImplSource::Future(future_data) => Some(Instance {
-            def: ty::InstanceDef::Item(future_data.generator_def_id),
-            substs: future_data.substs,
-        }),
+        traits::ImplSource::Generator(generator_data) => {
+            if cfg!(debug_assertions) && tcx.item_name(trait_item_id) != sym::resume {
+                // For compiler developers who'd like to add new items to `Generator`,
+                // you either need to generate a shim body, or perhaps return
+                // `InstanceDef::Item` pointing to a trait default method body if
+                // it is given a default implementation by the trait.
+                span_bug!(
+                    tcx.def_span(generator_data.generator_def_id),
+                    "no definition for `{trait_ref}::{}` for built-in generator type",
+                    tcx.item_name(trait_item_id)
+                )
+            }
+            Some(Instance {
+                def: ty::InstanceDef::Item(generator_data.generator_def_id),
+                substs: generator_data.substs,
+            })
+        }
+        traits::ImplSource::Future(future_data) => {
+            if cfg!(debug_assertions) && tcx.item_name(trait_item_id) != sym::poll {
+                // For compiler developers who'd like to add new items to `Future`,
+                // you either need to generate a shim body, or perhaps return
+                // `InstanceDef::Item` pointing to a trait default method body if
+                // it is given a default implementation by the trait.
+                span_bug!(
+                    tcx.def_span(future_data.generator_def_id),
+                    "no definition for `{trait_ref}::{}` for built-in async generator type",
+                    tcx.item_name(trait_item_id)
+                )
+            }
+            Some(Instance {
+                def: ty::InstanceDef::Item(future_data.generator_def_id),
+                substs: future_data.substs,
+            })
+        }
         traits::ImplSource::Closure(closure_data) => {
+            if cfg!(debug_assertions)
+                && ![sym::call, sym::call_mut, sym::call_once]
+                    .contains(&tcx.item_name(trait_item_id))
+            {
+                // For compiler developers who'd like to add new items to `Fn`/`FnMut`/`FnOnce`,
+                // you either need to generate a shim body, or perhaps return
+                // `InstanceDef::Item` pointing to a trait default method body if
+                // it is given a default implementation by the trait.
+                span_bug!(
+                    tcx.def_span(closure_data.closure_def_id),
+                    "no definition for `{trait_ref}::{}` for built-in closure type",
+                    tcx.item_name(trait_item_id)
+                )
+            }
             let trait_closure_kind = tcx.fn_trait_kind_from_def_id(trait_id).unwrap();
             Instance::resolve_closure(
                 tcx,
@@ -195,11 +235,29 @@ fn resolve_associated_item<'tcx>(
             )
         }
         traits::ImplSource::FnPointer(ref data) => match data.fn_ty.kind() {
-            ty::FnDef(..) | ty::FnPtr(..) => Some(Instance {
-                def: ty::InstanceDef::FnPtrShim(trait_item_id, data.fn_ty),
-                substs: rcvr_substs,
-            }),
-            _ => None,
+            ty::FnDef(..) | ty::FnPtr(..) => {
+                if cfg!(debug_assertions)
+                    && ![sym::call, sym::call_mut, sym::call_once]
+                        .contains(&tcx.item_name(trait_item_id))
+                {
+                    // For compiler developers who'd like to add new items to `Fn`/`FnMut`/`FnOnce`,
+                    // you either need to generate a shim body, or perhaps return
+                    // `InstanceDef::Item` pointing to a trait default method body if
+                    // it is given a default implementation by the trait.
+                    bug!(
+                        "no definition for `{trait_ref}::{}` for built-in fn type",
+                        tcx.item_name(trait_item_id)
+                    )
+                }
+                Some(Instance {
+                    def: ty::InstanceDef::FnPtrShim(trait_item_id, data.fn_ty),
+                    substs: rcvr_substs,
+                })
+            }
+            _ => bug!(
+                "no built-in definition for `{trait_ref}::{}` for non-fn type",
+                tcx.item_name(trait_item_id)
+            ),
         },
         traits::ImplSource::Object(ref data) => {
             traits::get_vtable_index_of_object_method(tcx, data, trait_item_id).map(|index| {
