From 154c90a087b41d8397a145b50c9aac3385e00142 Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Wed, 22 Mar 2023 13:32:24 +0300 Subject: [PATCH 1/2] correctly handle normalization in implied bounds --- .../query/type_op/implied_outlives_bounds.rs | 118 ++++++------------ .../issue-111404-1.rs | 1 + .../issue-111404-1.stderr | 10 +- .../normalization-nested.lifetime.stderr | 18 --- .../ui/implied-bounds/normalization-nested.rs | 7 +- tests/ui/inference/issue-80409.rs | 5 +- 6 files changed, 52 insertions(+), 107 deletions(-) delete mode 100644 tests/ui/implied-bounds/normalization-nested.lifetime.stderr diff --git a/compiler/rustc_trait_selection/src/traits/query/type_op/implied_outlives_bounds.rs b/compiler/rustc_trait_selection/src/traits/query/type_op/implied_outlives_bounds.rs index 56844f39478fb..e42009ccac91d 100644 --- a/compiler/rustc_trait_selection/src/traits/query/type_op/implied_outlives_bounds.rs +++ b/compiler/rustc_trait_selection/src/traits/query/type_op/implied_outlives_bounds.rs @@ -9,8 +9,6 @@ use rustc_infer::traits::query::OutlivesBound; use rustc_middle::infer::canonical::CanonicalQueryResponse; use rustc_middle::traits::ObligationCause; use rustc_middle::ty::{self, ParamEnvAnd, Ty, TyCtxt, TypeVisitableExt}; -use rustc_span::def_id::CRATE_DEF_ID; -use rustc_span::DUMMY_SP; use smallvec::{smallvec, SmallVec}; #[derive(Copy, Clone, Debug, HashStable, TypeFoldable, TypeVisitable)] @@ -58,12 +56,29 @@ impl<'tcx> super::QueryTypeOp<'tcx> for ImpliedOutlivesBounds<'tcx> { } } +/// For the sake of completeness, we should be careful when dealing with inference artifacts: +/// - `ty` must be fully resolved. +/// - `normalize_op` must return a fully resolved type. pub fn compute_implied_outlives_bounds_inner<'tcx>( ocx: &ObligationCtxt<'_, 'tcx>, param_env: ty::ParamEnv<'tcx>, ty: Ty<'tcx>, ) -> Result>, NoSolution> { - let tcx = ocx.infcx.tcx; + let normalize_op = |ty: Ty<'tcx>| { + let ty = if ocx.infcx.next_trait_solver() { + solve::deeply_normalize(ocx.infcx.at(&ObligationCause::dummy(), param_env), ty) + // FIXME(-Ztrait-solver=next) + .unwrap_or_else(|_errs| ty) + } else { + ocx.normalize(&ObligationCause::dummy(), param_env, ty) + }; + if !ocx.select_all_or_error().is_empty() { + return Err(NoSolution); + } + let ty = ocx.infcx.resolve_vars_if_possible(ty); + assert!(!ty.has_non_region_infer()); + Ok(ty) + }; // Sometimes when we ask what it takes for T: WF, we get back that // U: WF is required; in that case, we push U onto this stack and @@ -71,56 +86,22 @@ pub fn compute_implied_outlives_bounds_inner<'tcx>( // guaranteed to be a subset of the original type, so we need to store the // WF args we've computed in a set. let mut checked_wf_args = rustc_data_structures::fx::FxHashSet::default(); - let mut wf_args = vec![ty.into()]; + let mut wf_args = vec![ty.into(), normalize_op(ty)?.into()]; - let mut outlives_bounds: Vec, ty::Region<'tcx>>> = - vec![]; + let mut outlives_bounds: Vec> = vec![]; while let Some(arg) = wf_args.pop() { if !checked_wf_args.insert(arg) { continue; } - // Compute the obligations for `arg` to be well-formed. If `arg` is - // an unresolved inference variable, just substituted an empty set - // -- because the return type here is going to be things we *add* - // to the environment, it's always ok for this set to be smaller - // than the ultimate set. (Note: normally there won't be - // unresolved inference variables here anyway, but there might be - // during typeck under some circumstances.) - // - // FIXME(@lcnr): It's not really "always fine", having fewer implied - // bounds can be backward incompatible, e.g. #101951 was caused by - // us not dealing with inference vars in `TypeOutlives` predicates. - let obligations = wf::obligations(ocx.infcx, param_env, CRATE_DEF_ID, 0, arg, DUMMY_SP) - .unwrap_or_default(); - - for obligation in obligations { - debug!(?obligation); + // From the full set of obligations, just filter down to the region relationships. + for obligation in + wf::unnormalized_obligations(ocx.infcx, param_env, arg).into_iter().flatten() + { assert!(!obligation.has_escaping_bound_vars()); - - // While these predicates should all be implied by other parts of - // the program, they are still relevant as they may constrain - // inference variables, which is necessary to add the correct - // implied bounds in some cases, mostly when dealing with projections. - // - // Another important point here: we only register `Projection` - // predicates, since otherwise we might register outlives - // predicates containing inference variables, and we don't - // learn anything new from those. - if obligation.predicate.has_non_region_infer() { - match obligation.predicate.kind().skip_binder() { - ty::PredicateKind::Clause(ty::ClauseKind::Projection(..)) - | ty::PredicateKind::AliasRelate(..) => { - ocx.register_obligation(obligation.clone()); - } - _ => {} - } - } - - let pred = match obligation.predicate.kind().no_bound_vars() { - None => continue, - Some(pred) => pred, + let Some(pred) = obligation.predicate.kind().no_bound_vars() else { + continue; }; match pred { ty::PredicateKind::Clause(ty::ClauseKind::Trait(..)) @@ -143,53 +124,24 @@ pub fn compute_implied_outlives_bounds_inner<'tcx>( } // We need to register region relationships - ty::PredicateKind::Clause(ty::ClauseKind::RegionOutlives(ty::OutlivesPredicate( - r_a, - r_b, - ))) => outlives_bounds.push(ty::OutlivesPredicate(r_a.into(), r_b)), + ty::PredicateKind::Clause(ty::ClauseKind::RegionOutlives( + ty::OutlivesPredicate(r_a, r_b), + )) => outlives_bounds.push(OutlivesBound::RegionSubRegion(r_b, r_a)), ty::PredicateKind::Clause(ty::ClauseKind::TypeOutlives(ty::OutlivesPredicate( ty_a, r_b, - ))) => outlives_bounds.push(ty::OutlivesPredicate(ty_a.into(), r_b)), - } - } - } - - // This call to `select_all_or_error` is necessary to constrain inference variables, which we - // use further down when computing the implied bounds. - match ocx.select_all_or_error().as_slice() { - [] => (), - _ => return Err(NoSolution), - } - - // We lazily compute the outlives components as - // `select_all_or_error` constrains inference variables. - let mut implied_bounds = Vec::new(); - for ty::OutlivesPredicate(a, r_b) in outlives_bounds { - match a.unpack() { - ty::GenericArgKind::Lifetime(r_a) => { - implied_bounds.push(OutlivesBound::RegionSubRegion(r_b, r_a)) - } - ty::GenericArgKind::Type(ty_a) => { - let mut ty_a = ocx.infcx.resolve_vars_if_possible(ty_a); - // Need to manually normalize in the new solver as `wf::obligations` does not. - if ocx.infcx.next_trait_solver() { - ty_a = solve::deeply_normalize( - ocx.infcx.at(&ObligationCause::dummy(), param_env), - ty_a, - ) - .map_err(|_errs| NoSolution)?; + ))) => { + let ty_a = normalize_op(ty_a)?; + let mut components = smallvec![]; + push_outlives_components(ocx.infcx.tcx, ty_a, &mut components); + outlives_bounds.extend(implied_bounds_from_components(r_b, components)) } - let mut components = smallvec![]; - push_outlives_components(tcx, ty_a, &mut components); - implied_bounds.extend(implied_bounds_from_components(r_b, components)) } - ty::GenericArgKind::Const(_) => unreachable!(), } } - Ok(implied_bounds) + Ok(outlives_bounds) } /// When we have an implied bound that `T: 'a`, we can further break diff --git a/tests/ui/associated-inherent-types/issue-111404-1.rs b/tests/ui/associated-inherent-types/issue-111404-1.rs index dd62e59f07d22..74f9434b8818a 100644 --- a/tests/ui/associated-inherent-types/issue-111404-1.rs +++ b/tests/ui/associated-inherent-types/issue-111404-1.rs @@ -10,5 +10,6 @@ impl<'a> Foo { fn bar(_: fn(Foo fn(Foo::Assoc)>::Assoc)) {} //~^ ERROR higher-ranked subtype error //~| ERROR higher-ranked subtype error +//~| ERROR higher-ranked subtype error fn main() {} diff --git a/tests/ui/associated-inherent-types/issue-111404-1.stderr b/tests/ui/associated-inherent-types/issue-111404-1.stderr index cf4d4a5f19b12..1613161a873f7 100644 --- a/tests/ui/associated-inherent-types/issue-111404-1.stderr +++ b/tests/ui/associated-inherent-types/issue-111404-1.stderr @@ -12,5 +12,13 @@ LL | fn bar(_: fn(Foo fn(Foo::Assoc)>::Assoc)) {} | = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` -error: aborting due to 2 previous errors +error: higher-ranked subtype error + --> $DIR/issue-111404-1.rs:10:1 + | +LL | fn bar(_: fn(Foo fn(Foo::Assoc)>::Assoc)) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error: aborting due to 3 previous errors diff --git a/tests/ui/implied-bounds/normalization-nested.lifetime.stderr b/tests/ui/implied-bounds/normalization-nested.lifetime.stderr deleted file mode 100644 index abffee57a0f09..0000000000000 --- a/tests/ui/implied-bounds/normalization-nested.lifetime.stderr +++ /dev/null @@ -1,18 +0,0 @@ -error[E0759]: `fn` parameter has lifetime `'x` but it needs to satisfy a `'static` lifetime requirement - --> $DIR/normalization-nested.rs:35:20 - | -LL | pub fn test<'x>(_: Map>, s: &'x str) -> &'static str { - | ^^^^^^^^^^^^^^^^ - | | - | this data with lifetime `'x`... - | ...is used and required to live as long as `'static` here - | -note: `'static` lifetime requirement introduced by this bound - --> $DIR/normalization-nested.rs:33:14 - | -LL | I::Item: 'static; - | ^^^^^^^ - -error: aborting due to 1 previous error - -For more information about this error, try `rustc --explain E0759`. diff --git a/tests/ui/implied-bounds/normalization-nested.rs b/tests/ui/implied-bounds/normalization-nested.rs index 5f1cbb3f69779..0935fe0f91b5c 100644 --- a/tests/ui/implied-bounds/normalization-nested.rs +++ b/tests/ui/implied-bounds/normalization-nested.rs @@ -1,11 +1,10 @@ // Test for normalization of projections that appear in the item bounds // (versus those that appear directly in the input types). -// Both revisions should pass. `lifetime` revision is a bug. +// Both revisions should pass. +// `lifetime` revision was incorrectly rejected. See #109799. // // revisions: param_ty lifetime -// [param_ty] check-pass -// [lifetime] check-fail -// [lifetime] known-bug: #109799 +// check-pass pub trait Iter { type Item; diff --git a/tests/ui/inference/issue-80409.rs b/tests/ui/inference/issue-80409.rs index 80cad6dfc46e4..3a1ea30af5cde 100644 --- a/tests/ui/inference/issue-80409.rs +++ b/tests/ui/inference/issue-80409.rs @@ -1,4 +1,7 @@ -// check-pass +// This is an ICE because of #104478. +// known-bug: unknown +// failure-status: 101 +// dont-check-compiler-stderr #![allow(unreachable_code, unused)] From b3e3b536c69093bb74c3f913268e70f005652e37 Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Thu, 23 Mar 2023 05:20:57 +0300 Subject: [PATCH 2/2] add test --- tests/ui/implied-bounds/from-trait-impl.rs | 21 +++++++++++++ .../ui/implied-bounds/from-trait-impl.stderr | 31 +++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 tests/ui/implied-bounds/from-trait-impl.rs create mode 100644 tests/ui/implied-bounds/from-trait-impl.stderr diff --git a/tests/ui/implied-bounds/from-trait-impl.rs b/tests/ui/implied-bounds/from-trait-impl.rs new file mode 100644 index 0000000000000..770c47d9d2077 --- /dev/null +++ b/tests/ui/implied-bounds/from-trait-impl.rs @@ -0,0 +1,21 @@ +// Foo> shouldn't imply X: 'static. +// We don't use region constraints from trait impls in implied bounds. + +trait Trait { + type Assoc; +} + +impl Trait for Vec { + type Assoc = (); +} + +struct Foo(T) +where + T::Assoc: 'static, // any predicate naming T::Assoc +; + +fn foo(_: Foo>) {} +//~^ ERROR `X` may not live long enough +//~| ERROR `X` may not live long enough + +fn main() {} diff --git a/tests/ui/implied-bounds/from-trait-impl.stderr b/tests/ui/implied-bounds/from-trait-impl.stderr new file mode 100644 index 0000000000000..bf5ce6d3a0166 --- /dev/null +++ b/tests/ui/implied-bounds/from-trait-impl.stderr @@ -0,0 +1,31 @@ +error[E0310]: the parameter type `X` may not live long enough + --> $DIR/from-trait-impl.rs:17:1 + | +LL | fn foo(_: Foo>) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | the parameter type `X` must be valid for the static lifetime... + | ...so that the type `X` will meet its required lifetime bounds + | +help: consider adding an explicit lifetime bound + | +LL | fn foo(_: Foo>) {} + | +++++++++ + +error[E0310]: the parameter type `X` may not live long enough + --> $DIR/from-trait-impl.rs:17:14 + | +LL | fn foo(_: Foo>) {} + | ^^^^^^^^^^^ + | | + | the parameter type `X` must be valid for the static lifetime... + | ...so that the type `X` will meet its required lifetime bounds + | +help: consider adding an explicit lifetime bound + | +LL | fn foo(_: Foo>) {} + | +++++++++ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0310`.