Revert "stop special-casing 'static in evaluate"

This reverts commit 73c79cd.

Author: lqd

compiler/rustc_infer/src/infer/freshen.rs

@@ -43,16 +43,18 @@ pub struct TypeFreshener<'a, 'tcx> {
     const_freshen_count: u32,
     ty_freshen_map: FxHashMap<ty::InferTy, Ty<'tcx>>,
     const_freshen_map: FxHashMap<ty::InferConst<'tcx>, ty::Const<'tcx>>,
+    keep_static: bool,
 }
 
 impl<'a, 'tcx> TypeFreshener<'a, 'tcx> {
-    pub fn new(infcx: &'a InferCtxt<'tcx>) -> TypeFreshener<'a, 'tcx> {
+    pub fn new(infcx: &'a InferCtxt<'tcx>, keep_static: bool) -> 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,
         }
     }
 
@@ -119,9 +121,18 @@ impl<'a, 'tcx> TypeFolder<TyCtxt<'tcx>> for TypeFreshener<'a, 'tcx> {
             | ty::ReFree(_)
             | ty::ReVar(_)
             | ty::RePlaceholder(..)
-            | ty::ReStatic
             | ty::ReError(_)
-            | ty::ReErased => self.interner().lifetimes.re_erased,
+            | 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
+                }
+            }
         }
     }
 

compiler/rustc_infer/src/infer/mod.rs

@@ -713,7 +713,12 @@ impl<'tcx> InferCtxt<'tcx> {
     }
 
     pub fn freshener<'b>(&'b self) -> TypeFreshener<'b, 'tcx> {
-        freshen::TypeFreshener::new(self)
+        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)
     }
 
     pub fn unsolved_variables(&self) -> Vec<Ty<'tcx>> {

compiler/rustc_trait_selection/src/traits/select/mod.rs

@@ -210,7 +210,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     pub fn new(infcx: &'cx InferCtxt<'tcx>) -> SelectionContext<'cx, 'tcx> {
         SelectionContext {
             infcx,
-            freshener: infcx.freshener(),
+            freshener: infcx.freshener_keep_static(),
             intercrate_ambiguity_causes: None,
             query_mode: TraitQueryMode::Standard,
         }
@@ -769,16 +769,14 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 }
 
                 ty::PredicateKind::Clause(ty::Clause::TypeOutlives(pred)) => {
-                    // 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 {
+                    // 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 {
+                        Ok(EvaluatedToOkModuloRegions)
                     }
                 }
 
@@ -1826,12 +1824,6 @@ 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
@@ -1897,7 +1889,11 @@ 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.
-                    DropVictim::drop_if(other.bound_vars().len() <= victim.bound_vars().len())
+                    if other.bound_vars().len() <= victim.bound_vars().len() {
+                        DropVictim::Yes
+                    } else {
+                        DropVictim::No
+                    }
                 } 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
@@ -1927,13 +1923,17 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
                 | ObjectCandidate(_)
                 | ProjectionCandidate(..),
             ) => {
-                // 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))
+                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
+                }
             }
             (ObjectCandidate(_) | ProjectionCandidate(..), ParamCandidate(ref victim_cand)) => {
                 // Prefer these to a global where-clause bound
@@ -1955,16 +1955,18 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
             ) => {
                 // Prefer these to a global where-clause bound
                 // (see issue #50825).
-                DropVictim::drop_if(
-                    is_global(victim_cand) && other.evaluation.must_apply_modulo_regions(),
-                )
+                if is_global(victim_cand) && other.evaluation.must_apply_modulo_regions() {
+                    DropVictim::Yes
+                } else {
+                    DropVictim::No
+                }
             }
 
             (ProjectionCandidate(i, _), ProjectionCandidate(j, _))
             | (ObjectCandidate(i), ObjectCandidate(j)) => {
                 // Arbitrarily pick the lower numbered candidate for backwards
                 // compatibility reasons. Don't let this affect inference.
-                DropVictim::drop_if(i < j && !needs_infer)
+                if i < j && !needs_infer { DropVictim::Yes } else { DropVictim::No }
             }
             (ObjectCandidate(_), ProjectionCandidate(..))
             | (ProjectionCandidate(..), ObjectCandidate(_)) => {
@@ -2015,65 +2017,55 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
                     }
                 }
 
-                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
-                    }
-                    // 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(),
-                        )
+                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,
                     }
-                    None => DropVictim::No,
+                } else {
+                    DropVictim::No
                 }
             }
 

tests/ui/marker_trait_attr/overlap-marker-trait-with-static-lifetime.rs

@@ -1,8 +1,4 @@
-// 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
+// check-pass
 #![feature(marker_trait_attr)]
 
 #[marker]

tests/ui/marker_trait_attr/overlap-marker-trait-with-static-lifetime.stderr

@@ -1,17 +1,9 @@
-// known-bug: #109481
-//
-// While the `T: Copy` is always applicable when checking
-// that the impl `impl<T: Copy> 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.
+// check-pass
 #![feature(marker_trait_attr)]
 
 #[marker]
 pub trait F {}
-impl<T: Copy> F for T {}
-impl<T: 'static> F for T {}
+impl<T> F for T where T: Copy {}
+impl<T> F for T where T: 'static {}
 
 fn main() {}