Rollup merge of rust-lang#124532 - lcnr:elaborate-coherence, r=compiler-errors

elaborate obligations in coherence

The following test currently does not pass coherence:
```rust
trait Super {}
trait Sub<T>: Super {}

trait Overlap<T> {}
impl<T, U: Sub<T>> Overlap<T> for U {}
impl<T> Overlap<T> for () {}

fn main() {}
```

We check whether `(): Sub<?t>` holds. This stalls with ambiguity as downstream crates may add an impl for `(): Sub<Local>`. However, its super trait bound `(): Super` cannot be implemented downstream, so this one is known not to hold.

By elaborating the bounds in the implicit negative overlap check, this now compiles. This is necessary to prevent breakage from enabling `-Znext-solver=coherence` (rust-lang#121848), see tests/ui/coherence/super-traits/super-trait-knowable-2.rs for more details.

r? ``@compiler-errors``

Author: matthiaskrgr

compiler/rustc_middle/src/traits/select.rs

@@ -121,7 +121,18 @@ pub enum SelectionCandidate<'tcx> {
     /// Implementation of transmutability trait.
     TransmutabilityCandidate,
 
-    ParamCandidate(ty::PolyTraitPredicate<'tcx>),
+    /// A candidate from the `ParamEnv`.
+    ParamCandidate {
+        /// The actual `where`-bound, e.g. `T: Trait`.
+        predicate: ty::PolyTraitPredicate<'tcx>,
+        /// `true` if the where-bound has no bound vars and does
+        /// not refer to any parameters or inference variables.
+        ///
+        /// We prefer all other candidates over global where-bounds.
+        /// Notably, global where-bounds do not shadow impls.
+        is_global: bool,
+    },
+
     ImplCandidate(DefId),
     AutoImplCandidate,
 

compiler/rustc_trait_selection/src/traits/coherence.rs

@@ -359,9 +359,12 @@ fn impl_intersection_has_impossible_obligation<'a, 'cx, 'tcx>(
 ) -> IntersectionHasImpossibleObligations<'tcx> {
     let infcx = selcx.infcx;
 
+    // Elaborate obligations in case the current obligation is unknowable,
+    // but its super trait bound is not. See #124532 for more details.
+    let obligations = util::elaborate(infcx.tcx, obligations.iter().cloned());
     if infcx.next_trait_solver() {
         let ocx = ObligationCtxt::new(infcx);
-        ocx.register_obligations(obligations.iter().cloned());
+        ocx.register_obligations(obligations);
         let errors_and_ambiguities = ocx.select_all_or_error();
         // We only care about the obligations that are *definitely* true errors.
         // Ambiguities do not prove the disjointness of two impls.
@@ -388,7 +391,7 @@ fn impl_intersection_has_impossible_obligation<'a, 'cx, 'tcx>(
         for obligation in obligations {
             // We use `evaluate_root_obligation` to correctly track intercrate
             // ambiguity clauses.
-            let evaluation_result = selcx.evaluate_root_obligation(obligation);
+            let evaluation_result = selcx.evaluate_root_obligation(&obligation);
 
             match evaluation_result {
                 Ok(result) => {

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

@@ -252,16 +252,18 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             all_bounds.filter(|p| p.def_id() == stack.obligation.predicate.def_id());
 
         // Keep only those bounds which may apply, and propagate overflow if it occurs.
-        for bound in matching_bounds {
-            if bound.skip_binder().polarity != stack.obligation.predicate.skip_binder().polarity {
+        for predicate in matching_bounds {
+            if predicate.skip_binder().polarity != stack.obligation.predicate.skip_binder().polarity
+            {
                 continue;
             }
 
             // FIXME(oli-obk): it is suspicious that we are dropping the constness and
             // polarity here.
-            let wc = self.where_clause_may_apply(stack, bound.map_bound(|t| t.trait_ref))?;
+            let wc = self.where_clause_may_apply(stack, predicate.map_bound(|t| t.trait_ref))?;
             if wc.may_apply() {
-                candidates.vec.push(ParamCandidate(bound));
+                let is_global = predicate.is_global() && !predicate.has_bound_vars();
+                candidates.vec.push(ParamCandidate { predicate, is_global });
             }
         }
 

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

@@ -57,9 +57,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 ImplSource::Builtin(BuiltinImplSource::Misc, data)
             }
 
-            ParamCandidate(param) => {
+            ParamCandidate { predicate, is_global: _ } => {
                 let obligations =
-                    self.confirm_param_candidate(obligation, param.map_bound(|t| t.trait_ref));
+                    self.confirm_param_candidate(obligation, predicate.map_bound(|t| t.trait_ref));
                 ImplSource::Param(obligations)
             }
 

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

@@ -1577,7 +1577,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             return false;
         }
         match result {
-            Ok(Some(SelectionCandidate::ParamCandidate(trait_ref))) => !trait_ref.has_infer(),
+            Ok(Some(SelectionCandidate::ParamCandidate { predicate, .. })) => {
+                !predicate.has_infer()
+            }
             _ => true,
         }
     }
@@ -1829,31 +1831,35 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
             return DropVictim::Yes;
         }
 
-        // Check if a bound would previously have been removed when normalizing
-        // the param_env so that it can be given the lowest priority. See
-        // #50825 for the motivation for this.
-        let is_global =
-            |cand: &ty::PolyTraitPredicate<'tcx>| cand.is_global() && !cand.has_bound_vars();
-
-        // (*) Prefer `BuiltinCandidate { has_nested: false }`, `PointeeCandidate`,
-        // `DiscriminantKindCandidate`, `ConstDestructCandidate`
-        // to anything else.
-        //
-        // This is a fix for #53123 and prevents winnowing from accidentally extending the
-        // lifetime of a variable.
         match (&other.candidate, &victim.candidate) {
-            // FIXME(@jswrenn): this should probably be more sophisticated
-            (TransmutabilityCandidate, _) | (_, TransmutabilityCandidate) => DropVictim::No,
-
-            // (*)
+            // Prefer `BuiltinCandidate { has_nested: false }`, `ConstDestructCandidate`
+            // to anything else.
+            //
+            // This is a fix for #53123 and prevents winnowing from accidentally extending the
+            // lifetime of a variable.
+            (
+                BuiltinCandidate { has_nested: false } | ConstDestructCandidate(_),
+                BuiltinCandidate { has_nested: false } | ConstDestructCandidate(_),
+            ) => bug!("two trivial builtin candidates: {other:?} {victim:?}"),
             (BuiltinCandidate { has_nested: false } | ConstDestructCandidate(_), _) => {
                 DropVictim::Yes
             }
             (_, BuiltinCandidate { has_nested: false } | ConstDestructCandidate(_)) => {
                 DropVictim::No
             }
 
-            (ParamCandidate(other), ParamCandidate(victim)) => {
+            // Global bounds from the where clause should be ignored
+            // here (see issue #50825).
+            (ParamCandidate { is_global: true, .. }, ParamCandidate { is_global: true, .. }) => {
+                DropVictim::No
+            }
+            (_, ParamCandidate { is_global: true, .. }) => DropVictim::Yes,
+            (ParamCandidate { is_global: true, .. }, _) => DropVictim::No,
+
+            (
+                ParamCandidate { is_global: false, predicate: other },
+                ParamCandidate { is_global: false, predicate: victim },
+            ) => {
                 let same_except_bound_vars = other.skip_binder().trait_ref
                     == victim.skip_binder().trait_ref
                     && other.skip_binder().polarity == victim.skip_binder().polarity
@@ -1870,68 +1876,8 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
                 }
             }
 
-            // Drop otherwise equivalent non-const fn pointer candidates
-            (FnPointerCandidate { .. }, FnPointerCandidate { fn_host_effect }) => {
-                DropVictim::drop_if(*fn_host_effect == self.tcx().consts.true_)
-            }
-
-            (
-                ParamCandidate(ref other_cand),
-                ImplCandidate(..)
-                | AutoImplCandidate
-                | ClosureCandidate { .. }
-                | AsyncClosureCandidate
-                | AsyncFnKindHelperCandidate
-                | CoroutineCandidate
-                | FutureCandidate
-                | IteratorCandidate
-                | AsyncIteratorCandidate
-                | FnPointerCandidate { .. }
-                | BuiltinObjectCandidate
-                | BuiltinUnsizeCandidate
-                | TraitUpcastingUnsizeCandidate(_)
-                | BuiltinCandidate { .. }
-                | TraitAliasCandidate
-                | 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))
-            }
-            (ObjectCandidate(_) | ProjectionCandidate(_), ParamCandidate(ref victim_cand)) => {
-                // Prefer these to a global where-clause bound
-                // (see issue #50825).
-                if is_global(victim_cand) { DropVictim::Yes } else { DropVictim::No }
-            }
-            (
-                ImplCandidate(_)
-                | AutoImplCandidate
-                | ClosureCandidate { .. }
-                | AsyncClosureCandidate
-                | AsyncFnKindHelperCandidate
-                | CoroutineCandidate
-                | FutureCandidate
-                | IteratorCandidate
-                | AsyncIteratorCandidate
-                | FnPointerCandidate { .. }
-                | BuiltinObjectCandidate
-                | BuiltinUnsizeCandidate
-                | TraitUpcastingUnsizeCandidate(_)
-                | BuiltinCandidate { has_nested: true }
-                | TraitAliasCandidate,
-                ParamCandidate(ref victim_cand),
-            ) => {
-                // Prefer these to a global where-clause bound
-                // (see issue #50825).
-                DropVictim::drop_if(
-                    is_global(victim_cand) && other.evaluation.must_apply_modulo_regions(),
-                )
-            }
+            (ParamCandidate { is_global: false, .. }, _) => DropVictim::Yes,
+            (_, ParamCandidate { is_global: false, .. }) => DropVictim::No,
 
             (ProjectionCandidate(i), ProjectionCandidate(j))
             | (ObjectCandidate(i), ObjectCandidate(j)) => {
@@ -1944,44 +1890,18 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
                 bug!("Have both object and projection candidate")
             }
 
-            // Arbitrarily give projection and object candidates priority.
-            (
-                ObjectCandidate(_) | ProjectionCandidate(_),
-                ImplCandidate(..)
-                | AutoImplCandidate
-                | ClosureCandidate { .. }
-                | AsyncClosureCandidate
-                | AsyncFnKindHelperCandidate
-                | CoroutineCandidate
-                | FutureCandidate
-                | IteratorCandidate
-                | AsyncIteratorCandidate
-                | FnPointerCandidate { .. }
-                | BuiltinObjectCandidate
-                | BuiltinUnsizeCandidate
-                | TraitUpcastingUnsizeCandidate(_)
-                | BuiltinCandidate { .. }
-                | TraitAliasCandidate,
-            ) => DropVictim::Yes,
+            // Arbitrarily give projection candidates priority.
+            (ProjectionCandidate(_), _) => DropVictim::Yes,
+            (_, ProjectionCandidate(_)) => DropVictim::No,
 
-            (
-                ImplCandidate(..)
-                | AutoImplCandidate
-                | ClosureCandidate { .. }
-                | AsyncClosureCandidate
-                | AsyncFnKindHelperCandidate
-                | CoroutineCandidate
-                | FutureCandidate
-                | IteratorCandidate
-                | AsyncIteratorCandidate
-                | FnPointerCandidate { .. }
-                | BuiltinObjectCandidate
-                | BuiltinUnsizeCandidate
-                | TraitUpcastingUnsizeCandidate(_)
-                | BuiltinCandidate { .. }
-                | TraitAliasCandidate,
-                ObjectCandidate(_) | ProjectionCandidate(_),
-            ) => DropVictim::No,
+            // Need to prioritize builtin trait object impls as
+            // `<dyn Any as Any>::type_id` should use the vtable method
+            // and not the method provided by the user-defined impl
+            // `impl<T: ?Sized> Any for T { .. }`.
+            //
+            // cc #57893
+            (ObjectCandidate(_), _) => DropVictim::Yes,
+            (_, ObjectCandidate(_)) => DropVictim::No,
 
             (&ImplCandidate(other_def), &ImplCandidate(victim_def)) => {
                 // See if we can toss out `victim` based on specialization.
@@ -2061,49 +1981,25 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
                 }
             }
 
-            (AutoImplCandidate, ImplCandidate(_)) | (ImplCandidate(_), AutoImplCandidate) => {
-                DropVictim::No
-            }
-
-            (AutoImplCandidate, _) | (_, AutoImplCandidate) => {
-                bug!(
-                    "default implementations shouldn't be recorded \
-                    when there are other global candidates: {:?} {:?}",
-                    other,
-                    victim
-                );
-            }
-
-            // Everything else is ambiguous
+            // Treat all non-trivial builtin impls and user-defined impls the same way.
             (
                 ImplCandidate(_)
-                | ClosureCandidate { .. }
-                | AsyncClosureCandidate
-                | AsyncFnKindHelperCandidate
-                | CoroutineCandidate
-                | FutureCandidate
-                | IteratorCandidate
-                | AsyncIteratorCandidate
-                | FnPointerCandidate { .. }
-                | BuiltinObjectCandidate
-                | BuiltinUnsizeCandidate
-                | TraitUpcastingUnsizeCandidate(_)
+                | AutoImplCandidate
                 | BuiltinCandidate { has_nested: true }
-                | TraitAliasCandidate,
-                ImplCandidate(_)
-                | ClosureCandidate { .. }
                 | AsyncClosureCandidate
                 | AsyncFnKindHelperCandidate
                 | CoroutineCandidate
                 | FutureCandidate
                 | IteratorCandidate
                 | AsyncIteratorCandidate
                 | FnPointerCandidate { .. }
-                | BuiltinObjectCandidate
+                | ClosureCandidate { .. }
+                | TraitAliasCandidate
                 | BuiltinUnsizeCandidate
                 | TraitUpcastingUnsizeCandidate(_)
-                | BuiltinCandidate { has_nested: true }
-                | TraitAliasCandidate,
+                | TransmutabilityCandidate
+                | BuiltinObjectCandidate,
+                _,
             ) => DropVictim::No,
         }
     }

src/tools/tidy/src/issues.txt

@@ -4061,7 +4061,6 @@ ui/traits/issue-6128.rs
 ui/traits/issue-6334.rs
 ui/traits/issue-65284-suggest-generic-trait-bound.rs
 ui/traits/issue-65673.rs
-ui/traits/issue-66768.rs
 ui/traits/issue-68295.rs
 ui/traits/issue-7013.rs
 ui/traits/issue-70944.rs

tests/ui/associated-item/issue-105449.rs

@@ -1,13 +1,14 @@
-//@ check-pass
 //@ compile-flags: -C debug_assertions=yes -Zunstable-options
 
-#[allow(dead_code)]
+// This is a mutated variant of #66768 which has been removed
+// as it no longer tests the original issue.
 fn problematic_function<Space>()
 where
     DefaultAlloc: FinAllok<R1, Space>,
 {
     let e = Edge2dElement;
     let _ = Into::<Point>::into(e.map_reference_coords());
+    //~^ ERROR the trait bound `Point: From<(Ure, R1, MStorage)>` is not satisfied
 }
 impl<N> Allocator<N, R0> for DefaultAlloc {
     type Buffer = MStorage;

tests/ui/associated-item/issue-105449.stderr

@@ -0,0 +1,15 @@
+error[E0277]: the trait bound `Point: From<(Ure, R1, MStorage)>` is not satisfied
+  --> $DIR/issue-105449.rs:10:33
+   |
+LL |     let _ = Into::<Point>::into(e.map_reference_coords());
+   |             ------------------- ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `From<(Ure, R1, MStorage)>` is not implemented for `Point`, which is required by `(Ure, R1, MStorage): Into<Point>`
+   |             |
+   |             required by a bound introduced by this call
+   |
+   = help: the trait `From<(Ure, Space, <DefaultAlloc as Allocator<Ure, Space>>::Buffer)>` is implemented for `Point`
+   = help: for that trait implementation, expected `Space`, found `R1`
+   = note: required for `(Ure, R1, MStorage)` to implement `Into<Point>`
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0277`.

tests/ui/coherence/coherence-overlap-unnormalizable-projection-0.classic.stderr

@@ -11,8 +11,8 @@ LL | |     for<'a> <T as WithAssoc<'a>>::Assoc: WhereBound,
 LL |   impl<T> Trait for Box<T> {}
    |   ^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `Box<_>`
    |
-   = note: downstream crates may implement trait `WithAssoc<'a>` for type `std::boxed::Box<_>`
    = note: downstream crates may implement trait `WhereBound` for type `<std::boxed::Box<_> as WithAssoc<'a>>::Assoc`
+   = note: downstream crates may implement trait `WithAssoc<'a>` for type `std::boxed::Box<_>`
 
 error: aborting due to 1 previous error
 

tests/ui/coherence/coherence-overlap-unnormalizable-projection-1.classic.stderr

@@ -11,8 +11,8 @@ LL | |     for<'a> Box<<T as WithAssoc<'a>>::Assoc>: WhereBound,
 LL |   impl<T> Trait for Box<T> {}
    |   ^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `Box<_>`
    |
-   = note: downstream crates may implement trait `WithAssoc<'a>` for type `std::boxed::Box<_>`
    = note: downstream crates may implement trait `WhereBound` for type `std::boxed::Box<<std::boxed::Box<_> as WithAssoc<'a>>::Assoc>`
+   = note: downstream crates may implement trait `WithAssoc<'a>` for type `std::boxed::Box<_>`
 
 error: aborting due to 1 previous error
 

tests/ui/coherence/normalize-for-errors.current.stderr

@@ -8,6 +8,7 @@ LL | impl<S: Iterator> MyTrait<S> for (Box<<(MyType,) as Mirror>::Assoc>, S::Ite
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `(Box<(MyType,)>, _)`
    |
    = note: upstream crates may add a new impl of trait `std::marker::Copy` for type `std::boxed::Box<(MyType,)>` in future versions
+   = note: upstream crates may add a new impl of trait `std::clone::Clone` for type `(MyType,)` in future versions
 
 error: aborting due to 1 previous error