Rollup merge of rust-lang#131699 - compiler-errors:better-errors-for-projections, r=lcnr

Try to improve error messages involving aliases in the solver

1. Treat aliases as rigid only if it may not be defined and it's well formed (i.e. for projections, its trait goal is satisfied).
2. Record goals that are related to alias normalization under a new `GoalKind`, so we can look into them in the `BestObligation` visitor.
3. Try to deduplicate errors due to self types of goals that are un-normalizable aliases.

r? lcnr

Author: matthiaskrgr

compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs

@@ -6,6 +6,7 @@ use derive_where::derive_where;
 use rustc_type_ir::fold::TypeFoldable;
 use rustc_type_ir::inherent::*;
 use rustc_type_ir::lang_items::TraitSolverLangItem;
+use rustc_type_ir::solve::inspect;
 use rustc_type_ir::visit::TypeVisitableExt as _;
 use rustc_type_ir::{self as ty, Interner, Upcast as _, elaborate};
 use tracing::{debug, instrument};
@@ -288,6 +289,25 @@ where
         let Ok(normalized_self_ty) =
             self.structurally_normalize_ty(goal.param_env, goal.predicate.self_ty())
         else {
+            // FIXME: We register a fake candidate when normalization fails so that
+            // we can point at the reason for *why*. I'm tempted to say that this
+            // is the wrong way to do this, though.
+            let result =
+                self.probe(|&result| inspect::ProbeKind::RigidAlias { result }).enter(|this| {
+                    let normalized_ty = this.next_ty_infer();
+                    let alias_relate_goal = Goal::new(
+                        this.cx(),
+                        goal.param_env,
+                        ty::PredicateKind::AliasRelate(
+                            goal.predicate.self_ty().into(),
+                            normalized_ty.into(),
+                            ty::AliasRelationDirection::Equate,
+                        ),
+                    );
+                    this.add_goal(GoalSource::AliasWellFormed, alias_relate_goal);
+                    this.evaluate_added_goals_and_make_canonical_response(Certainty::AMBIGUOUS)
+                });
+            assert_eq!(result, Err(NoSolution));
             return vec![];
         };
 

compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs

@@ -983,7 +983,7 @@ where
             hidden_ty,
             &mut goals,
         );
-        self.add_goals(GoalSource::Misc, goals);
+        self.add_goals(GoalSource::AliasWellFormed, goals);
     }
 
     // Do something for each opaque/hidden pair defined with `def_id` in the

compiler/rustc_next_trait_solver/src/solve/normalizes_to/mod.rs

@@ -15,7 +15,7 @@ use crate::solve::assembly::{self, Candidate};
 use crate::solve::inspect::ProbeKind;
 use crate::solve::{
     BuiltinImplSource, CandidateSource, Certainty, EvalCtxt, Goal, GoalSource, MaybeCause,
-    NoSolution, QueryResult,
+    NoSolution, QueryResult, Reveal,
 };
 
 impl<D, I> EvalCtxt<'_, D>
@@ -37,10 +37,61 @@ where
         match normalize_result {
             Ok(res) => Ok(res),
             Err(NoSolution) => {
-                let Goal { param_env, predicate: NormalizesTo { alias, term } } = goal;
-                self.relate_rigid_alias_non_alias(param_env, alias, ty::Invariant, term)?;
-                self.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
+                self.probe(|&result| ProbeKind::RigidAlias { result }).enter(|this| {
+                    let Goal { param_env, predicate: NormalizesTo { alias, term } } = goal;
+                    this.add_rigid_constraints(param_env, alias)?;
+                    this.relate_rigid_alias_non_alias(param_env, alias, ty::Invariant, term)?;
+                    this.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
+                })
+            }
+        }
+    }
+
+    /// Register any obligations that are used to validate that an alias should be
+    /// treated as rigid.
+    ///
+    /// An alias may be considered rigid if it fails normalization, but we also don't
+    /// want to consider aliases that are not well-formed to be rigid simply because
+    /// they fail normalization.
+    ///
+    /// For example, some `<T as Trait>::Assoc` where `T: Trait` does not hold, or an
+    /// opaque type whose hidden type doesn't actually satisfy the opaque item bounds.
+    fn add_rigid_constraints(
+        &mut self,
+        param_env: I::ParamEnv,
+        rigid_alias: ty::AliasTerm<I>,
+    ) -> Result<(), NoSolution> {
+        let cx = self.cx();
+        match rigid_alias.kind(cx) {
+            // Projections are rigid only if their trait ref holds,
+            // and the GAT where-clauses hold.
+            ty::AliasTermKind::ProjectionTy | ty::AliasTermKind::ProjectionConst => {
+                let trait_ref = rigid_alias.trait_ref(cx);
+                self.add_goal(GoalSource::AliasWellFormed, Goal::new(cx, param_env, trait_ref));
+                Ok(())
+            }
+            ty::AliasTermKind::OpaqueTy => {
+                match param_env.reveal() {
+                    // In user-facing mode, paques are only rigid if we may not define it.
+                    Reveal::UserFacing => {
+                        if rigid_alias
+                            .def_id
+                            .as_local()
+                            .is_some_and(|def_id| self.can_define_opaque_ty(def_id))
+                        {
+                            Err(NoSolution)
+                        } else {
+                            Ok(())
+                        }
+                    }
+                    // Opaques are never rigid in reveal-all mode.
+                    Reveal::All => Err(NoSolution),
+                }
             }
+            // FIXME(generic_const_exprs): we would need to support generic consts here
+            ty::AliasTermKind::UnevaluatedConst => Err(NoSolution),
+            // Inherent and weak types are never rigid. This type must not be well-formed.
+            ty::AliasTermKind::WeakTy | ty::AliasTermKind::InherentTy => Err(NoSolution),
         }
     }
 
@@ -124,6 +175,7 @@ where
                     ecx.instantiate_normalizes_to_term(goal, assumption_projection_pred.term);
 
                     // Add GAT where clauses from the trait's definition
+                    // FIXME: We don't need these, since these are the type's own WF obligations.
                     ecx.add_goals(
                         GoalSource::Misc,
                         cx.own_predicates_of(goal.predicate.def_id())
@@ -179,7 +231,8 @@ where
                 .map(|pred| goal.with(cx, pred));
             ecx.add_goals(GoalSource::ImplWhereBound, where_clause_bounds);
 
-            // Add GAT where clauses from the trait's definition
+            // Add GAT where clauses from the trait's definition.
+            // FIXME: We don't need these, since these are the type's own WF obligations.
             ecx.add_goals(
                 GoalSource::Misc,
                 cx.own_predicates_of(goal.predicate.def_id())

compiler/rustc_trait_selection/src/solve/fulfill.rs

@@ -14,7 +14,7 @@ use rustc_middle::bug;
 use rustc_middle::ty::error::{ExpectedFound, TypeError};
 use rustc_middle::ty::{self, TyCtxt};
 use rustc_next_trait_solver::solve::{GenerateProofTree, HasChanged, SolverDelegateEvalExt as _};
-use tracing::instrument;
+use tracing::{instrument, trace};
 
 use super::Certainty;
 use super::delegate::SolverDelegate;
@@ -401,6 +401,7 @@ impl<'tcx> BestObligation<'tcx> {
                                         nested_goal.source(),
                                         GoalSource::ImplWhereBound
                                             | GoalSource::InstantiateHigherRanked
+                                            | GoalSource::AliasWellFormed
                                     ) && match self.consider_ambiguities {
                                         true => {
                                             matches!(
@@ -415,6 +416,13 @@ impl<'tcx> BestObligation<'tcx> {
                         })
                     });
                 }
+
+                // Prefer a non-rigid candidate if there is one.
+                if candidates.len() > 1 {
+                    candidates.retain(|candidate| {
+                        !matches!(candidate.kind(), inspect::ProbeKind::RigidAlias { .. })
+                    });
+                }
             }
         }
 
@@ -429,8 +437,11 @@ impl<'tcx> ProofTreeVisitor<'tcx> for BestObligation<'tcx> {
         self.obligation.cause.span
     }
 
+    #[instrument(level = "trace", skip(self, goal), fields(goal = ?goal.goal()))]
     fn visit_goal(&mut self, goal: &inspect::InspectGoal<'_, 'tcx>) -> Self::Result {
         let candidates = self.non_trivial_candidates(goal);
+        trace!(candidates = ?candidates.iter().map(|c| c.kind()).collect::<Vec<_>>());
+
         let [candidate] = candidates.as_slice() else {
             return ControlFlow::Break(self.obligation.clone());
         };
@@ -464,17 +475,13 @@ impl<'tcx> ProofTreeVisitor<'tcx> for BestObligation<'tcx> {
                     polarity: ty::PredicatePolarity::Positive,
                 }))
             }
-            ty::PredicateKind::Clause(
-                ty::ClauseKind::WellFormed(_) | ty::ClauseKind::Projection(..),
-            )
-            | ty::PredicateKind::AliasRelate(..) => ChildMode::PassThrough,
-            _ => {
-                return ControlFlow::Break(self.obligation.clone());
-            }
+            _ => ChildMode::PassThrough,
         };
 
         let mut impl_where_bound_count = 0;
         for nested_goal in candidate.instantiate_nested_goals(self.span()) {
+            trace!(nested_goal = ?(nested_goal.goal(), nested_goal.source(), nested_goal.result()));
+
             let make_obligation = |cause| Obligation {
                 cause,
                 param_env: nested_goal.goal().param_env,
@@ -501,7 +508,7 @@ impl<'tcx> ProofTreeVisitor<'tcx> for BestObligation<'tcx> {
                 (_, GoalSource::InstantiateHigherRanked) => {
                     obligation = self.obligation.clone();
                 }
-                (ChildMode::PassThrough, _) => {
+                (ChildMode::PassThrough, _) | (_, GoalSource::AliasWellFormed) => {
                     obligation = make_obligation(self.obligation.cause.clone());
                 }
             }

compiler/rustc_trait_selection/src/solve/inspect/analyse.rs

@@ -292,7 +292,8 @@ impl<'a, 'tcx> InspectGoal<'a, 'tcx> {
                         | inspect::ProbeKind::Root { .. }
                         | inspect::ProbeKind::TryNormalizeNonRigid { .. }
                         | inspect::ProbeKind::TraitCandidate { .. }
-                        | inspect::ProbeKind::OpaqueTypeStorageLookup { .. } => {
+                        | inspect::ProbeKind::OpaqueTypeStorageLookup { .. }
+                        | inspect::ProbeKind::RigidAlias { .. } => {
                             // Nested probes have to prove goals added in their parent
                             // but do not leak them, so we truncate the added goals
                             // afterwards.
@@ -316,7 +317,8 @@ impl<'a, 'tcx> InspectGoal<'a, 'tcx> {
             inspect::ProbeKind::Root { result }
             | inspect::ProbeKind::TryNormalizeNonRigid { result }
             | inspect::ProbeKind::TraitCandidate { source: _, result }
-            | inspect::ProbeKind::OpaqueTypeStorageLookup { result } => {
+            | inspect::ProbeKind::OpaqueTypeStorageLookup { result }
+            | inspect::ProbeKind::RigidAlias { result } => {
                 // We only add a candidate if `shallow_certainty` was set, which means
                 // that we ended up calling `evaluate_added_goals_and_make_canonical_response`.
                 if let Some(shallow_certainty) = shallow_certainty {

compiler/rustc_trait_selection/src/solve/select.rs

@@ -177,7 +177,8 @@ fn to_selection<'tcx>(
         | ProbeKind::UpcastProjectionCompatibility
         | ProbeKind::OpaqueTypeStorageLookup { result: _ }
         | ProbeKind::Root { result: _ }
-        | ProbeKind::ShadowedEnvProbing => {
+        | ProbeKind::ShadowedEnvProbing
+        | ProbeKind::RigidAlias { result: _ } => {
             span_bug!(span, "didn't expect to assemble trait candidate from {:#?}", cand.kind())
         }
     })

compiler/rustc_type_ir/src/solve/inspect.rs

@@ -135,4 +135,6 @@ pub enum ProbeKind<I: Interner> {
     ShadowedEnvProbing,
     /// Try to unify an opaque type with an existing key in the storage.
     OpaqueTypeStorageLookup { result: QueryResult<I> },
+    /// Checking that a rigid alias is well-formed.
+    RigidAlias { result: QueryResult<I> },
 }

compiler/rustc_type_ir/src/solve/mod.rs

@@ -130,6 +130,10 @@ pub enum GoalSource {
     ImplWhereBound,
     /// Instantiating a higher-ranked goal and re-proving it.
     InstantiateHigherRanked,
+    /// Predicate required for an alias projection to be well-formed.
+    /// This is used in two places: projecting to an opaque whose hidden type
+    /// is already registered in the opaque type storage, and for rigid projections.
+    AliasWellFormed,
 }
 
 #[derive_where(Clone; I: Interner, Goal<I, P>: Clone)]

tests/ui/for/issue-20605.next.stderr

@@ -11,13 +11,6 @@ help: consider mutably borrowing here
 LL |     for item in &mut *things { *item = 0 }
    |                 ++++
 
-error[E0614]: type `<dyn Iterator<Item = &'a mut u8> as IntoIterator>::Item` cannot be dereferenced
-  --> $DIR/issue-20605.rs:6:27
-   |
-LL |     for item in *things { *item = 0 }
-   |                           ^^^^^
-
-error: aborting due to 2 previous errors
+error: aborting due to 1 previous error
 
-Some errors have detailed explanations: E0277, E0614.
-For more information about an error, try `rustc --explain E0277`.
+For more information about this error, try `rustc --explain E0277`.

tests/ui/for/issue-20605.rs

@@ -4,12 +4,7 @@
 
 fn changer<'a>(mut things: Box<dyn Iterator<Item=&'a mut u8>>) {
     for item in *things { *item = 0 }
-    //[current]~^ ERROR `dyn Iterator<Item = &'a mut u8>` is not an iterator
-    //[next]~^^ ERROR `dyn Iterator<Item = &'a mut u8>` is not an iterator
-    //[next]~| ERROR type `<dyn Iterator<Item = &'a mut u8> as IntoIterator>::Item` cannot be dereferenced
-
-    // FIXME(-Znext-solver): these error messages are horrible and have to be
-    // improved before we stabilize the new solver.
+    //~^ ERROR `dyn Iterator<Item = &'a mut u8>` is not an iterator
 }
 
 fn main() {}