Skip to content

Commit

Permalink
Auto merge of #96806 - cjgillot:codegen-fulfill-nice, r=oli-obk
Browse files Browse the repository at this point in the history
Gracefully fail to resolve associated items instead of `delay_span_bug`.

`codegen_fulfill_obligation` is used during instance resolution for trait items.

In case of insufficient normalization issues during MIR inlining, it caused ICEs.
It's better to gracefully refuse to resolve the associated item, and let the caller decide what to do with this.

Split from #91743
Closes #69121
Closes #73021
Closes #88599
Closes #93008
Closes #93248
Closes #94680
Closes #96170
r? `@oli-obk`
  • Loading branch information
bors committed May 11, 2022
2 parents 6dd6840 + dacf118 commit cb9cb4d
Show file tree
Hide file tree
Showing 12 changed files with 108 additions and 88 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1145,7 +1145,7 @@ rustc_queries! {

query codegen_fulfill_obligation(
key: (ty::ParamEnv<'tcx>, ty::PolyTraitRef<'tcx>)
) -> Result<&'tcx ImplSource<'tcx, ()>, ErrorGuaranteed> {
) -> Result<&'tcx ImplSource<'tcx, ()>, traits::CodegenObligationError> {
cache_on_disk_if { true }
desc { |tcx|
"checking if `{}` fulfills its obligations",
Expand Down
18 changes: 18 additions & 0 deletions compiler/rustc_middle/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -963,3 +963,21 @@ pub enum MethodViolationCode {
/// the method's receiver (`self` argument) can't be dispatched on
UndispatchableReceiver,
}

/// These are the error cases for `codegen_fulfill_obligation`.
#[derive(Copy, Clone, Debug, Hash, HashStable, Encodable, Decodable)]
pub enum CodegenObligationError {
/// Ambiguity can happen when monomorphizing during trans
/// expands to some humongous type that never occurred
/// statically -- this humongous type can then overflow,
/// leading to an ambiguous result. So report this as an
/// overflow bug, since I believe this is the only case
/// where ambiguity can result.
Ambiguity,
/// This can trigger when we probe for the source of a `'static` lifetime requirement
/// on a trait object: `impl Foo for dyn Trait {}` has an implicit `'static` bound.
/// This can also trigger when we have a global bound that is not actually satisfied,
/// but was included during typeck due to the trivial_bounds feature.
Unimplemented,
FulfillmentError,
}
91 changes: 16 additions & 75 deletions compiler/rustc_trait_selection/src/traits/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,12 @@
// seems likely that they should eventually be merged into more
// general routines.

use crate::infer::{InferCtxt, TyCtxtInferExt};
use crate::infer::TyCtxtInferExt;
use crate::traits::{
FulfillmentContext, ImplSource, Obligation, ObligationCause, SelectionContext, TraitEngine,
Unimplemented,
};
use rustc_errors::ErrorGuaranteed;
use rustc_middle::ty::fold::TypeFoldable;
use rustc_middle::traits::CodegenObligationError;
use rustc_middle::ty::{self, TyCtxt};

/// Attempts to resolve an obligation to an `ImplSource`. The result is
Expand All @@ -23,7 +22,7 @@ use rustc_middle::ty::{self, TyCtxt};
pub fn codegen_fulfill_obligation<'tcx>(
tcx: TyCtxt<'tcx>,
(param_env, trait_ref): (ty::ParamEnv<'tcx>, ty::PolyTraitRef<'tcx>),
) -> Result<&'tcx ImplSource<'tcx, ()>, ErrorGuaranteed> {
) -> Result<&'tcx ImplSource<'tcx, ()>, CodegenObligationError> {
// Remove any references to regions; this helps improve caching.
let trait_ref = tcx.erase_regions(trait_ref);
// We expect the input to be fully normalized.
Expand All @@ -40,37 +39,8 @@ pub fn codegen_fulfill_obligation<'tcx>(

let selection = match selcx.select(&obligation) {
Ok(Some(selection)) => selection,
Ok(None) => {
// Ambiguity can happen when monomorphizing during trans
// expands to some humongous type that never occurred
// statically -- this humongous type can then overflow,
// leading to an ambiguous result. So report this as an
// overflow bug, since I believe this is the only case
// where ambiguity can result.
let reported = infcx.tcx.sess.delay_span_bug(
rustc_span::DUMMY_SP,
&format!(
"encountered ambiguity selecting `{:?}` during codegen, presuming due to \
overflow or prior type error",
trait_ref
),
);
return Err(reported);
}
Err(Unimplemented) => {
// This can trigger when we probe for the source of a `'static` lifetime requirement
// on a trait object: `impl Foo for dyn Trait {}` has an implicit `'static` bound.
// This can also trigger when we have a global bound that is not actually satisfied,
// but was included during typeck due to the trivial_bounds feature.
let guar = infcx.tcx.sess.delay_span_bug(
rustc_span::DUMMY_SP,
&format!(
"Encountered error `Unimplemented` selecting `{:?}` during codegen",
trait_ref
),
);
return Err(guar);
}
Ok(None) => return Err(CodegenObligationError::Ambiguity),
Err(Unimplemented) => return Err(CodegenObligationError::Unimplemented),
Err(e) => {
bug!("Encountered error `{:?}` selecting `{:?}` during codegen", e, trait_ref)
}
Expand All @@ -85,7 +55,17 @@ pub fn codegen_fulfill_obligation<'tcx>(
let impl_source = selection.map(|predicate| {
fulfill_cx.register_predicate_obligation(&infcx, predicate);
});
let impl_source = drain_fulfillment_cx_or_panic(&infcx, &mut fulfill_cx, impl_source);

// In principle, we only need to do this so long as `impl_source`
// contains unbound type parameters. It could be a slight
// optimization to stop iterating early.
let errors = fulfill_cx.select_all_or_error(&infcx);
if !errors.is_empty() {
return Err(CodegenObligationError::FulfillmentError);
}

let impl_source = infcx.resolve_vars_if_possible(impl_source);
let impl_source = infcx.tcx.erase_regions(impl_source);

// Opaque types may have gotten their hidden types constrained, but we can ignore them safely
// as they will get constrained elsewhere, too.
Expand All @@ -95,42 +75,3 @@ pub fn codegen_fulfill_obligation<'tcx>(
Ok(&*tcx.arena.alloc(impl_source))
})
}

// # Global Cache

/// Finishes processes any obligations that remain in the
/// fulfillment context, and then returns the result with all type
/// variables removed and regions erased. Because this is intended
/// for use outside of type inference, if any errors occur,
/// it will panic. It is used during normalization and other cases
/// where processing the obligations in `fulfill_cx` may cause
/// type inference variables that appear in `result` to be
/// unified, and hence we need to process those obligations to get
/// the complete picture of the type.
fn drain_fulfillment_cx_or_panic<'tcx, T>(
infcx: &InferCtxt<'_, 'tcx>,
fulfill_cx: &mut FulfillmentContext<'tcx>,
result: T,
) -> T
where
T: TypeFoldable<'tcx>,
{
debug!("drain_fulfillment_cx_or_panic()");

// In principle, we only need to do this so long as `result`
// contains unbound type parameters. It could be a slight
// optimization to stop iterating early.
let errors = fulfill_cx.select_all_or_error(infcx);
if !errors.is_empty() {
infcx.tcx.sess.delay_span_bug(
rustc_span::DUMMY_SP,
&format!(
"Encountered errors `{:?}` resolving bounds outside of type inference",
errors
),
);
}

let result = infcx.resolve_vars_if_possible(result);
infcx.tcx.erase_regions(result)
}
18 changes: 17 additions & 1 deletion compiler/rustc_ty_utils/src/instance.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use rustc_errors::ErrorGuaranteed;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_infer::infer::TyCtxtInferExt;
use rustc_middle::traits::CodegenObligationError;
use rustc_middle::ty::subst::SubstsRef;
use rustc_middle::ty::{self, Binder, Instance, Ty, TyCtxt, TypeFoldable, TypeVisitor};
use rustc_span::{sym, DUMMY_SP};
Expand Down Expand Up @@ -212,7 +213,22 @@ fn resolve_associated_item<'tcx>(
let mut bound_vars_collector = BoundVarsCollector::new();
trait_ref.visit_with(&mut bound_vars_collector);
let trait_binder = ty::Binder::bind_with_vars(trait_ref, bound_vars_collector.into_vars(tcx));
let vtbl = tcx.codegen_fulfill_obligation((param_env, trait_binder))?;
let vtbl = match tcx.codegen_fulfill_obligation((param_env, trait_binder)) {
Ok(vtbl) => vtbl,
Err(CodegenObligationError::Ambiguity) => {
let reported = tcx.sess.delay_span_bug(
tcx.def_span(trait_item_id),
&format!(
"encountered ambiguity selecting `{:?}` during codegen, presuming due to \
overflow or prior type error",
trait_binder
),
);
return Err(reported);
}
Err(CodegenObligationError::Unimplemented) => return Ok(None),
Err(CodegenObligationError::FulfillmentError) => return Ok(None),
};

// Now that we know which impl is being used, we can dispatch to
// the actual function:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ fn writes_to_path<C>(cap: &C) {
writes_to_specific_path(&cap);
//~^ ERROR: the trait bound `(): _Contains<&C>` is not satisfied [E0277]
//~| ERROR: unconstrained generic constant
//~| ERROR: mismatched types [E0308]
}

fn writes_to_specific_path<C: Delegates<()>>(cap: &C) {}
Expand Down
18 changes: 14 additions & 4 deletions src/test/ui/const-generics/generic_const_exprs/issue-85848.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ note: required because of the requirements on the impl of `Delegates<()>` for `&
LL | impl<T, U> Delegates<U> for T where T: Contains<U, true> {}
| ^^^^^^^^^^^^ ^
note: required by a bound in `writes_to_specific_path`
--> $DIR/issue-85848.rs:29:31
--> $DIR/issue-85848.rs:30:31
|
LL | fn writes_to_specific_path<C: Delegates<()>>(cap: &C) {}
| ^^^^^^^^^^^^^ required by this bound in `writes_to_specific_path`
Expand All @@ -43,11 +43,21 @@ note: required because of the requirements on the impl of `Delegates<()>` for `&
LL | impl<T, U> Delegates<U> for T where T: Contains<U, true> {}
| ^^^^^^^^^^^^ ^
note: required by a bound in `writes_to_specific_path`
--> $DIR/issue-85848.rs:29:31
--> $DIR/issue-85848.rs:30:31
|
LL | fn writes_to_specific_path<C: Delegates<()>>(cap: &C) {}
| ^^^^^^^^^^^^^ required by this bound in `writes_to_specific_path`

error: aborting due to 2 previous errors
error[E0308]: mismatched types
--> $DIR/issue-85848.rs:24:5
|
LL | writes_to_specific_path(&cap);
| ^^^^^^^^^^^^^^^^^^^^^^^ expected `true`, found `{ contains::<T, U>() }`
|
= note: expected type `true`
found type `{ contains::<T, U>() }`

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0277`.
Some errors have detailed explanations: E0277, E0308.
For more information about an error, try `rustc --explain E0277`.
1 change: 1 addition & 0 deletions src/test/ui/const-generics/issues/issue-86530.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ where
fn unit_literals() {
z(" ");
//~^ ERROR: the trait bound `&str: X` is not satisfied
//~| ERROR: unconstrained generic constant
}

fn main() {}
18 changes: 17 additions & 1 deletion src/test/ui/const-generics/issues/issue-86530.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,22 @@ LL | where
LL | T: X,
| ^ required by this bound in `z`

error: aborting due to previous error
error: unconstrained generic constant
--> $DIR/issue-86530.rs:16:5
|
LL | z(" ");
| ^
|
= help: try adding a `where` bound using this expression: `where [(); T::Y]:`
note: required by a bound in `z`
--> $DIR/issue-86530.rs:11:10
|
LL | fn z<T>(t: T)
| - required by a bound in this
...
LL | [(); T::Y]: ,
| ^^^^ required by this bound in `z`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0277`.
1 change: 1 addition & 0 deletions src/test/ui/issues/issue-77919.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
fn main() {
[1; <Multiply<Five, Five>>::VAL];
//~^ ERROR: constant expression depends on a generic parameter
}
trait TypeVal<T> {
const VAL: T;
Expand Down
16 changes: 12 additions & 4 deletions src/test/ui/issues/issue-77919.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0412]: cannot find type `PhantomData` in this scope
--> $DIR/issue-77919.rs:9:9
--> $DIR/issue-77919.rs:10:9
|
LL | _n: PhantomData,
| ^^^^^^^^^^^ not found in this scope
Expand All @@ -10,23 +10,31 @@ LL | use std::marker::PhantomData;
|

error[E0412]: cannot find type `VAL` in this scope
--> $DIR/issue-77919.rs:11:63
--> $DIR/issue-77919.rs:12:63
|
LL | impl<N, M> TypeVal<usize> for Multiply<N, M> where N: TypeVal<VAL> {}
| - ^^^ not found in this scope
| |
| help: you might be missing a type parameter: `, VAL`

error[E0046]: not all trait items implemented, missing: `VAL`
--> $DIR/issue-77919.rs:11:1
--> $DIR/issue-77919.rs:12:1
|
LL | const VAL: T;
| ------------- `VAL` from trait
...
LL | impl<N, M> TypeVal<usize> for Multiply<N, M> where N: TypeVal<VAL> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `VAL` in implementation

error: aborting due to 3 previous errors
error: constant expression depends on a generic parameter
--> $DIR/issue-77919.rs:2:9
|
LL | [1; <Multiply<Five, Five>>::VAL];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this may fail depending on what value the parameter takes

error: aborting due to 4 previous errors

Some errors have detailed explanations: E0046, E0412.
For more information about an error, try `rustc --explain E0046`.
2 changes: 1 addition & 1 deletion src/test/ui/recursion/issue-83150.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// build-fail
//~^ overflow evaluating
//~^ ERROR overflow evaluating the requirement

fn main() {
let mut iter = 0u8..1;
Expand Down
10 changes: 9 additions & 1 deletion src/tools/clippy/tests/ui/crashes/ice-6252.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,15 @@ LL | const VAL: T;
LL | impl<N, M> TypeVal<usize> for Multiply<N, M> where N: TypeVal<VAL> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `VAL` in implementation

error: aborting due to 3 previous errors
error: constant expression depends on a generic parameter
--> $DIR/ice-6252.rs:13:9
|
LL | [1; <Multiply<Five, Five>>::VAL];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this may fail depending on what value the parameter takes

error: aborting due to 4 previous errors

Some errors have detailed explanations: E0046, E0412.
For more information about an error, try `rustc --explain E0046`.

0 comments on commit cb9cb4d

Please sign in to comment.