Skip to content

Commit

Permalink
Rollup merge of #111279 - compiler-errors:core-item-resolve, r=cjgillot
Browse files Browse the repository at this point in the history
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
  • Loading branch information
matthiaskrgr authored May 6, 2023
2 parents 83b29ec + 2a1ef34 commit 3cb1a46
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 14 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1207,6 +1207,7 @@ symbols! {
require,
residual,
result,
resume,
return_position_impl_trait_in_trait,
return_type_notation,
rhs,
Expand Down
84 changes: 71 additions & 13 deletions compiler/rustc_ty_utils/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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| {
Expand Down

0 comments on commit 3cb1a46

Please sign in to comment.