diff --git a/compiler/rustc_infer/src/infer/freshen.rs b/compiler/rustc_infer/src/infer/freshen.rs index f09f93abf45d6..d89f63e5c53e9 100644 --- a/compiler/rustc_infer/src/infer/freshen.rs +++ b/compiler/rustc_infer/src/infer/freshen.rs @@ -43,18 +43,16 @@ pub struct TypeFreshener<'a, 'tcx> { const_freshen_count: u32, ty_freshen_map: FxHashMap>, const_freshen_map: FxHashMap, ty::Const<'tcx>>, - keep_static: bool, } impl<'a, 'tcx> TypeFreshener<'a, 'tcx> { - pub fn new(infcx: &'a InferCtxt<'tcx>, keep_static: bool) -> TypeFreshener<'a, 'tcx> { + pub fn new(infcx: &'a InferCtxt<'tcx>) -> TypeFreshener<'a, 'tcx> { TypeFreshener { infcx, ty_freshen_count: 0, const_freshen_count: 0, ty_freshen_map: Default::default(), const_freshen_map: Default::default(), - keep_static, } } @@ -121,18 +119,9 @@ impl<'a, 'tcx> TypeFolder> for TypeFreshener<'a, 'tcx> { | ty::ReFree(_) | ty::ReVar(_) | ty::RePlaceholder(..) + | ty::ReStatic | ty::ReError(_) - | ty::ReErased => { - // replace all free regions with 'erased - self.interner().lifetimes.re_erased - } - ty::ReStatic => { - if self.keep_static { - r - } else { - self.interner().lifetimes.re_erased - } - } + | ty::ReErased => self.interner().lifetimes.re_erased, } } diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index 9afe9cc1e760e..5ee560837780a 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -713,12 +713,7 @@ impl<'tcx> InferCtxt<'tcx> { } pub fn freshener<'b>(&'b self) -> TypeFreshener<'b, 'tcx> { - freshen::TypeFreshener::new(self, false) - } - - /// Like `freshener`, but does not replace `'static` regions. - pub fn freshener_keep_static<'b>(&'b self) -> TypeFreshener<'b, 'tcx> { - freshen::TypeFreshener::new(self, true) + freshen::TypeFreshener::new(self) } pub fn unsolved_variables(&self) -> Vec> { diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index 4f429f018edf7..98c3e7c13ac6e 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -211,7 +211,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { pub fn new(infcx: &'cx InferCtxt<'tcx>) -> SelectionContext<'cx, 'tcx> { SelectionContext { infcx, - freshener: infcx.freshener_keep_static(), + freshener: infcx.freshener(), intercrate_ambiguity_causes: None, query_mode: TraitQueryMode::Standard, } @@ -770,14 +770,16 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { } ty::PredicateKind::Clause(ty::Clause::TypeOutlives(pred)) => { - // A global type with no late-bound regions can only - // contain the "'static" lifetime (any other lifetime - // would either be late-bound or local), so it is guaranteed - // to outlive any other lifetime - if pred.0.is_global() && !pred.0.has_late_bound_vars() { - Ok(EvaluatedToOk) - } else { + // A global type with no free lifetimes or generic parameters + // outlives anything. + if pred.0.has_free_regions() + || pred.0.has_late_bound_regions() + || pred.0.has_non_region_infer() + || pred.0.has_non_region_infer() + { Ok(EvaluatedToOkModuloRegions) + } else { + Ok(EvaluatedToOk) } } @@ -1825,6 +1827,12 @@ enum DropVictim { No, } +impl DropVictim { + fn drop_if(should_drop: bool) -> DropVictim { + if should_drop { DropVictim::Yes } else { DropVictim::No } + } +} + /// ## Winnowing /// /// Winnowing is the process of attempting to resolve ambiguity by @@ -1890,11 +1898,7 @@ impl<'tcx> SelectionContext<'_, 'tcx> { // or the current one if tied (they should both evaluate to the same answer). This is // probably best characterized as a "hack", since we might prefer to just do our // best to *not* create essentially duplicate candidates in the first place. - if other.bound_vars().len() <= victim.bound_vars().len() { - DropVictim::Yes - } else { - DropVictim::No - } + DropVictim::drop_if(other.bound_vars().len() <= victim.bound_vars().len()) } else if other.skip_binder().trait_ref == victim.skip_binder().trait_ref && victim.skip_binder().constness == ty::BoundConstness::NotConst && other.skip_binder().polarity == victim.skip_binder().polarity @@ -1924,17 +1928,13 @@ impl<'tcx> SelectionContext<'_, 'tcx> { | ObjectCandidate(_) | ProjectionCandidate(..), ) => { - if is_global(other_cand) { - DropVictim::No - } else { - // We have a where clause so don't go around looking - // for impls. Arbitrarily give param candidates priority - // over projection and object candidates. - // - // Global bounds from the where clause should be ignored - // here (see issue #50825). - DropVictim::Yes - } + // We have a where clause so don't go around looking + // for impls. Arbitrarily give param candidates priority + // over projection and object candidates. + // + // Global bounds from the where clause should be ignored + // here (see issue #50825). + DropVictim::drop_if(!is_global(other_cand)) } (ObjectCandidate(_) | ProjectionCandidate(..), ParamCandidate(ref victim_cand)) => { // Prefer these to a global where-clause bound @@ -1956,18 +1956,16 @@ impl<'tcx> SelectionContext<'_, 'tcx> { ) => { // Prefer these to a global where-clause bound // (see issue #50825). - if is_global(victim_cand) && other.evaluation.must_apply_modulo_regions() { - DropVictim::Yes - } else { - DropVictim::No - } + DropVictim::drop_if( + is_global(victim_cand) && other.evaluation.must_apply_modulo_regions(), + ) } (ProjectionCandidate(i, _), ProjectionCandidate(j, _)) | (ObjectCandidate(i), ObjectCandidate(j)) => { // Arbitrarily pick the lower numbered candidate for backwards // compatibility reasons. Don't let this affect inference. - if i < j && !needs_infer { DropVictim::Yes } else { DropVictim::No } + DropVictim::drop_if(i < j && !needs_infer) } (ObjectCandidate(_), ProjectionCandidate(..)) | (ProjectionCandidate(..), ObjectCandidate(_)) => { @@ -2018,55 +2016,65 @@ impl<'tcx> SelectionContext<'_, 'tcx> { } } - if other.evaluation.must_apply_considering_regions() { - match tcx.impls_are_allowed_to_overlap(other_def, victim_def) { - Some(ty::ImplOverlapKind::Permitted { marker: true }) => { - // Subtle: If the predicate we are evaluating has inference - // variables, do *not* allow discarding candidates due to - // marker trait impls. - // - // Without this restriction, we could end up accidentally - // constraining inference variables based on an arbitrarily - // chosen trait impl. - // - // Imagine we have the following code: - // - // ```rust - // #[marker] trait MyTrait {} - // impl MyTrait for u8 {} - // impl MyTrait for bool {} - // ``` - // - // And we are evaluating the predicate `<_#0t as MyTrait>`. - // - // During selection, we will end up with one candidate for each - // impl of `MyTrait`. If we were to discard one impl in favor - // of the other, we would be left with one candidate, causing - // us to "successfully" select the predicate, unifying - // _#0t with (for example) `u8`. - // - // However, we have no reason to believe that this unification - // is correct - we've essentially just picked an arbitrary - // *possibility* for _#0t, and required that this be the *only* - // possibility. - // - // Eventually, we will either: - // 1) Unify all inference variables in the predicate through - // some other means (e.g. type-checking of a function). We will - // then be in a position to drop marker trait candidates - // without constraining inference variables (since there are - // none left to constrain) - // 2) Be left with some unconstrained inference variables. We - // will then correctly report an inference error, since the - // existence of multiple marker trait impls tells us nothing - // about which one should actually apply. - if needs_infer { DropVictim::No } else { DropVictim::Yes } - } - Some(_) => DropVictim::Yes, - None => DropVictim::No, + match tcx.impls_are_allowed_to_overlap(other_def, victim_def) { + // For #33140 the impl headers must be exactly equal, the trait must not have + // any associated items and there are no where-clauses. + // + // We can just arbitrarily drop one of the impls. + Some(ty::ImplOverlapKind::Issue33140) => { + assert_eq!(other.evaluation, victim.evaluation); + DropVictim::Yes } - } else { - DropVictim::No + // For candidates which already reference errors it doesn't really + // matter what we do 🤷 + Some(ty::ImplOverlapKind::Permitted { marker: false }) => { + DropVictim::drop_if(other.evaluation.must_apply_considering_regions()) + } + Some(ty::ImplOverlapKind::Permitted { marker: true }) => { + // Subtle: If the predicate we are evaluating has inference + // variables, do *not* allow discarding candidates due to + // marker trait impls. + // + // Without this restriction, we could end up accidentally + // constraining inference variables based on an arbitrarily + // chosen trait impl. + // + // Imagine we have the following code: + // + // ```rust + // #[marker] trait MyTrait {} + // impl MyTrait for u8 {} + // impl MyTrait for bool {} + // ``` + // + // And we are evaluating the predicate `<_#0t as MyTrait>`. + // + // During selection, we will end up with one candidate for each + // impl of `MyTrait`. If we were to discard one impl in favor + // of the other, we would be left with one candidate, causing + // us to "successfully" select the predicate, unifying + // _#0t with (for example) `u8`. + // + // However, we have no reason to believe that this unification + // is correct - we've essentially just picked an arbitrary + // *possibility* for _#0t, and required that this be the *only* + // possibility. + // + // Eventually, we will either: + // 1) Unify all inference variables in the predicate through + // some other means (e.g. type-checking of a function). We will + // then be in a position to drop marker trait candidates + // without constraining inference variables (since there are + // none left to constrain) + // 2) Be left with some unconstrained inference variables. We + // will then correctly report an inference error, since the + // existence of multiple marker trait impls tells us nothing + // about which one should actually apply. + DropVictim::drop_if( + !needs_infer && other.evaluation.must_apply_considering_regions(), + ) + } + None => DropVictim::No, } } diff --git a/tests/ui/marker_trait_attr/overlap-marker-trait-with-static-lifetime.rs b/tests/ui/marker_trait_attr/overlap-marker-trait-with-static-lifetime.rs index 62aa22d41ed8e..b9f1de7ec13a5 100644 --- a/tests/ui/marker_trait_attr/overlap-marker-trait-with-static-lifetime.rs +++ b/tests/ui/marker_trait_attr/overlap-marker-trait-with-static-lifetime.rs @@ -1,4 +1,8 @@ -// check-pass +// known-bug: #89515 +// +// The trait solver cannot deal with ambiguous marker trait impls +// if there are lifetimes involved. As we must not special-case any +// regions this does not work, even with 'static #![feature(marker_trait_attr)] #[marker] diff --git a/tests/ui/marker_trait_attr/overlap-marker-trait-with-static-lifetime.stderr b/tests/ui/marker_trait_attr/overlap-marker-trait-with-static-lifetime.stderr new file mode 100644 index 0000000000000..fe4de540b513a --- /dev/null +++ b/tests/ui/marker_trait_attr/overlap-marker-trait-with-static-lifetime.stderr @@ -0,0 +1,31 @@ +error[E0283]: type annotations needed: cannot satisfy `&'static (): Marker` + --> $DIR/overlap-marker-trait-with-static-lifetime.rs:11:17 + | +LL | impl Marker for &'static () {} + | ^^^^^^^^^^^ + | +note: multiple `impl`s satisfying `&'static (): Marker` found + --> $DIR/overlap-marker-trait-with-static-lifetime.rs:11:1 + | +LL | impl Marker for &'static () {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | impl Marker for &'static () {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error[E0283]: type annotations needed: cannot satisfy `&'static (): Marker` + --> $DIR/overlap-marker-trait-with-static-lifetime.rs:12:17 + | +LL | impl Marker for &'static () {} + | ^^^^^^^^^^^ + | +note: multiple `impl`s satisfying `&'static (): Marker` found + --> $DIR/overlap-marker-trait-with-static-lifetime.rs:11:1 + | +LL | impl Marker for &'static () {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | impl Marker for &'static () {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0283`. diff --git a/tests/ui/marker_trait_attr/overlapping-impl-1-modulo-regions.rs b/tests/ui/marker_trait_attr/overlapping-impl-1-modulo-regions.rs index a8f3db5f5b25b..97a814f51eec5 100644 --- a/tests/ui/marker_trait_attr/overlapping-impl-1-modulo-regions.rs +++ b/tests/ui/marker_trait_attr/overlapping-impl-1-modulo-regions.rs @@ -1,9 +1,17 @@ -// check-pass +// known-bug: #109481 +// +// While the `T: Copy` is always applicable when checking +// that the impl `impl F for T {}` is well formed, +// the old trait solver can only approximate this by checking +// that there are no inference variables in the obligation and +// no region constraints in the evaluation result. +// +// Because of this we end up with ambiguity here. #![feature(marker_trait_attr)] #[marker] pub trait F {} -impl F for T where T: Copy {} -impl F for T where T: 'static {} +impl F for T {} +impl F for T {} fn main() {} diff --git a/tests/ui/marker_trait_attr/overlapping-impl-1-modulo-regions.stderr b/tests/ui/marker_trait_attr/overlapping-impl-1-modulo-regions.stderr new file mode 100644 index 0000000000000..e713d1451cfd2 --- /dev/null +++ b/tests/ui/marker_trait_attr/overlapping-impl-1-modulo-regions.stderr @@ -0,0 +1,14 @@ +error[E0310]: the parameter type `T` may not live long enough + --> $DIR/overlapping-impl-1-modulo-regions.rs:14:21 + | +LL | impl F for T {} + | ^ ...so that the type `T` will meet its required lifetime bounds + | +help: consider adding an explicit lifetime bound... + | +LL | impl F for T {} + | +++++++++ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0310`.