Rollup merge of #110673 - compiler-errors:alias-bounds-2, r=lcnr

matthiaskrgr · web-flow · commit c14d912cd261 · 2023-05-10T06:12:13.000+02:00
Make alias bounds sound in the new solver (take 2) Make alias bounds sound in the new solver (in a way that does not require coinduction) by only considering them for projection types whose corresponding trait refs come from a param-env candidate. That is, given `<T as Trait>::Assoc: Bound`, we only *really* need to consider the alias bound if `T: Trait` is satisfied via a param-env candidate. If it's instead satisfied, e.g., via an user provided impl candidate or a , then that impl should have a concrete type to which we could otherwise normalize `<T as Trait>::Assoc`, and that concrete type is then responsible to prove the `Bound` on it. Similar consideration is given to opaque types, since we only need to consider alias bounds if we're *not* in reveal-all mode, since similarly we'd be able to reveal the opaque types and prove any bounds that way. This does not remove that hacky "eager projection replacement" logic from object bounds, which are somewhat like alias bounds. But removing this eager normalization behavior (added in #108333) would require full coinduction to be enabled. Compare to #110628, which does remove this object-bound custom logic but requires coinduction to be sound. r? `@lcnr`
diff --git a/compiler/rustc_trait_selection/src/solve/assembly/mod.rs b/compiler/rustc_trait_selection/src/solve/assembly/mod.rs
@@ -8,6 +8,7 @@ use rustc_data_structures::fx::FxIndexSet;
 use rustc_hir::def_id::DefId;
 use rustc_infer::traits::query::NoSolution;
 use rustc_infer::traits::util::elaborate;
+use rustc_infer::traits::Reveal;
 use rustc_middle::traits::solve::{CanonicalResponse, Certainty, Goal, MaybeCause, QueryResult};
 use rustc_middle::ty::fast_reject::TreatProjections;
 use rustc_middle::ty::TypeFoldable;
@@ -87,7 +88,9 @@ pub(super) enum CandidateSource {
 }
 
 /// Methods used to assemble candidates for either trait or projection goals.
-pub(super) trait GoalKind<'tcx>: TypeFoldable<TyCtxt<'tcx>> + Copy + Eq {
+pub(super) trait GoalKind<'tcx>:
+    TypeFoldable<TyCtxt<'tcx>> + Copy + Eq + std::fmt::Display
+{
     fn self_ty(self) -> Ty<'tcx>;
 
     fn trait_ref(self, tcx: TyCtxt<'tcx>) -> ty::TraitRef<'tcx>;
@@ -96,6 +99,17 @@ pub(super) trait GoalKind<'tcx>: TypeFoldable<TyCtxt<'tcx>> + Copy + Eq {
 
     fn trait_def_id(self, tcx: TyCtxt<'tcx>) -> DefId;
 
+    // Try equating an assumption predicate against a goal's predicate. If it
+    // holds, then execute the `then` callback, which should do any additional
+    // work, then produce a response (typically by executing
+    // [`EvalCtxt::evaluate_added_goals_and_make_canonical_response`]).
+    fn probe_and_match_goal_against_assumption(
+        ecx: &mut EvalCtxt<'_, 'tcx>,
+        goal: Goal<'tcx, Self>,
+        assumption: ty::Predicate<'tcx>,
+        then: impl FnOnce(&mut EvalCtxt<'_, 'tcx>) -> QueryResult<'tcx>,
+    ) -> QueryResult<'tcx>;
+
     // Consider a clause, which consists of a "assumption" and some "requirements",
     // to satisfy a goal. If the requirements hold, then attempt to satisfy our
     // goal by equating it with the assumption.
@@ -104,7 +118,26 @@ pub(super) trait GoalKind<'tcx>: TypeFoldable<TyCtxt<'tcx>> + Copy + Eq {
         goal: Goal<'tcx, Self>,
         assumption: ty::Predicate<'tcx>,
         requirements: impl IntoIterator<Item = Goal<'tcx, ty::Predicate<'tcx>>>,
-    ) -> QueryResult<'tcx>;
+    ) -> QueryResult<'tcx> {
+        Self::probe_and_match_goal_against_assumption(ecx, goal, assumption, |ecx| {
+            ecx.add_goals(requirements);
+            ecx.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
+        })
+    }
+
+    /// Consider a bound originating from the item bounds of an alias. For this we
+    /// require that the well-formed requirements of the self type of the goal
+    /// are "satisfied from the param-env".
+    /// See [`EvalCtxt::validate_alias_bound_self_from_param_env`].
+    fn consider_alias_bound_candidate(
+        ecx: &mut EvalCtxt<'_, 'tcx>,
+        goal: Goal<'tcx, Self>,
+        assumption: ty::Predicate<'tcx>,
+    ) -> QueryResult<'tcx> {
+        Self::probe_and_match_goal_against_assumption(ecx, goal, assumption, |ecx| {
+            ecx.validate_alias_bound_self_from_param_env(goal)
+        })
+    }
 
     // Consider a clause specifically for a `dyn Trait` self type. This requires
     // additionally checking all of the supertraits and object bounds to hold,
@@ -113,7 +146,25 @@ pub(super) trait GoalKind<'tcx>: TypeFoldable<TyCtxt<'tcx>> + Copy + Eq {
         ecx: &mut EvalCtxt<'_, 'tcx>,
         goal: Goal<'tcx, Self>,
         assumption: ty::Predicate<'tcx>,
-    ) -> QueryResult<'tcx>;
+    ) -> QueryResult<'tcx> {
+        Self::probe_and_match_goal_against_assumption(ecx, goal, assumption, |ecx| {
+            let tcx = ecx.tcx();
+            let ty::Dynamic(bounds, _, _) = *goal.predicate.self_ty().kind() else {
+                    bug!("expected object type in `consider_object_bound_candidate`");
+                };
+            ecx.add_goals(
+                structural_traits::predicates_for_object_candidate(
+                    &ecx,
+                    goal.param_env,
+                    goal.predicate.trait_ref(tcx),
+                    bounds,
+                )
+                .into_iter()
+                .map(|pred| goal.with(tcx, pred)),
+            );
+            ecx.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
+        })
+    }
 
     fn consider_impl_candidate(
         ecx: &mut EvalCtxt<'_, 'tcx>,
@@ -463,7 +514,7 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
 
         for assumption in self.tcx().item_bounds(alias_ty.def_id).subst(self.tcx(), alias_ty.substs)
         {
-            match G::consider_implied_clause(self, goal, assumption, []) {
+            match G::consider_alias_bound_candidate(self, goal, assumption) {
                 Ok(result) => {
                     candidates.push(Candidate { source: CandidateSource::AliasBound, result })
                 }
@@ -472,6 +523,105 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
         }
     }
 
+    /// Check that we are allowed to use an alias bound originating from the self
+    /// type of this goal. This means something different depending on the self type's
+    /// alias kind.
+    ///
+    /// * Projection: Given a goal with a self type such as `<Ty as Trait>::Assoc`,
+    /// we require that the bound `Ty: Trait` can be proven using either a nested alias
+    /// bound candidate, or a param-env candidate.
+    ///
+    /// * Opaque: The param-env must be in `Reveal::UserFacing` mode. Otherwise,
+    /// the goal should be proven by using the hidden type instead.
+    #[instrument(level = "debug", skip(self), ret)]
+    pub(super) fn validate_alias_bound_self_from_param_env<G: GoalKind<'tcx>>(
+        &mut self,
+        goal: Goal<'tcx, G>,
+    ) -> QueryResult<'tcx> {
+        match *goal.predicate.self_ty().kind() {
+            ty::Alias(ty::Projection, projection_ty) => {
+                let mut param_env_candidates = vec![];
+                let self_trait_ref = projection_ty.trait_ref(self.tcx());
+
+                if self_trait_ref.self_ty().is_ty_var() {
+                    return self
+                        .evaluate_added_goals_and_make_canonical_response(Certainty::AMBIGUOUS);
+                }
+
+                let trait_goal: Goal<'_, ty::TraitPredicate<'tcx>> = goal.with(
+                    self.tcx(),
+                    ty::TraitPredicate {
+                        trait_ref: self_trait_ref,
+                        constness: ty::BoundConstness::NotConst,
+                        polarity: ty::ImplPolarity::Positive,
+                    },
+                );
+
+                self.assemble_param_env_candidates(trait_goal, &mut param_env_candidates);
+                // FIXME: We probably need some sort of recursion depth check here.
+                // Can't come up with an example yet, though, and the worst case
+                // we can have is a compiler stack overflow...
+                self.assemble_alias_bound_candidates(trait_goal, &mut param_env_candidates);
+
+                // FIXME: We must also consider alias-bound candidates for a peculiar
+                // class of built-in candidates that I'll call "defaulted" built-ins.
+                //
+                // For example, we always know that `T: Pointee` is implemented, but
+                // we do not always know what `<T as Pointee>::Metadata` actually is,
+                // similar to if we had a user-defined impl with a `default type ...`.
+                // For these traits, since we're not able to always normalize their
+                // associated types to a concrete type, we must consider their alias bounds
+                // instead, so we can prove bounds such as `<T as Pointee>::Metadata: Copy`.
+                self.assemble_alias_bound_candidates_for_builtin_impl_default_items(
+                    trait_goal,
+                    &mut param_env_candidates,
+                );
+
+                self.merge_candidates(param_env_candidates)
+            }
+            ty::Alias(ty::Opaque, _opaque_ty) => match goal.param_env.reveal() {
+                Reveal::UserFacing => {
+                    self.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
+                }
+                Reveal::All => return Err(NoSolution),
+            },
+            _ => bug!("only expected to be called on alias tys"),
+        }
+    }
+
+    /// Assemble a subset of builtin impl candidates for a class of candidates called
+    /// "defaulted" built-in traits.
+    ///
+    /// For example, we always know that `T: Pointee` is implemented, but we do not
+    /// always know what `<T as Pointee>::Metadata` actually is! See the comment in
+    /// [`EvalCtxt::validate_alias_bound_self_from_param_env`] for more detail.
+    #[instrument(level = "debug", skip_all)]
+    fn assemble_alias_bound_candidates_for_builtin_impl_default_items<G: GoalKind<'tcx>>(
+        &mut self,
+        goal: Goal<'tcx, G>,
+        candidates: &mut Vec<Candidate<'tcx>>,
+    ) {
+        let lang_items = self.tcx().lang_items();
+        let trait_def_id = goal.predicate.trait_def_id(self.tcx());
+
+        // You probably shouldn't add anything to this list unless you
+        // know what you're doing.
+        let result = if lang_items.pointee_trait() == Some(trait_def_id) {
+            G::consider_builtin_pointee_candidate(self, goal)
+        } else if lang_items.discriminant_kind_trait() == Some(trait_def_id) {
+            G::consider_builtin_discriminant_kind_candidate(self, goal)
+        } else {
+            Err(NoSolution)
+        };
+
+        match result {
+            Ok(result) => {
+                candidates.push(Candidate { source: CandidateSource::BuiltinImpl, result })
+            }
+            Err(NoSolution) => (),
+        }
+    }
+
     #[instrument(level = "debug", skip_all)]
     fn assemble_object_bound_candidates<G: GoalKind<'tcx>>(
         &mut self,
diff --git a/compiler/rustc_trait_selection/src/solve/project_goals.rs b/compiler/rustc_trait_selection/src/solve/project_goals.rs
@@ -56,11 +56,11 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> {
         self.trait_def_id(tcx)
     }
 
-    fn consider_implied_clause(
+    fn probe_and_match_goal_against_assumption(
         ecx: &mut EvalCtxt<'_, 'tcx>,
         goal: Goal<'tcx, Self>,
         assumption: ty::Predicate<'tcx>,
-        requirements: impl IntoIterator<Item = Goal<'tcx, ty::Predicate<'tcx>>>,
+        then: impl FnOnce(&mut EvalCtxt<'_, 'tcx>) -> QueryResult<'tcx>,
     ) -> QueryResult<'tcx> {
         if let Some(poly_projection_pred) = assumption.to_opt_poly_projection_pred()
             && poly_projection_pred.projection_def_id() == goal.predicate.def_id()
@@ -75,49 +75,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> {
                 )?;
                 ecx.eq(goal.param_env, goal.predicate.term, assumption_projection_pred.term)
                     .expect("expected goal term to be fully unconstrained");
-                ecx.add_goals(requirements);
-                ecx.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
-            })
-        } else {
-            Err(NoSolution)
-        }
-    }
-
-    fn consider_object_bound_candidate(
-        ecx: &mut EvalCtxt<'_, 'tcx>,
-        goal: Goal<'tcx, Self>,
-        assumption: ty::Predicate<'tcx>,
-    ) -> QueryResult<'tcx> {
-        if let Some(poly_projection_pred) = assumption.to_opt_poly_projection_pred()
-            && poly_projection_pred.projection_def_id() == goal.predicate.def_id()
-        {
-            ecx.probe(|ecx| {
-                let tcx = ecx.tcx();
-
-                let assumption_projection_pred =
-                    ecx.instantiate_binder_with_infer(poly_projection_pred);
-                ecx.eq(
-                    goal.param_env,
-                    goal.predicate.projection_ty,
-                    assumption_projection_pred.projection_ty,
-                )?;
-
-                let ty::Dynamic(bounds, _, _) = *goal.predicate.self_ty().kind() else {
-                    bug!("expected object type in `consider_object_bound_candidate`");
-                };
-                ecx.add_goals(
-                    structural_traits::predicates_for_object_candidate(
-                        &ecx,
-                        goal.param_env,
-                        goal.predicate.projection_ty.trait_ref(tcx),
-                        bounds,
-                    )
-                    .into_iter()
-                    .map(|pred| goal.with(tcx, pred)),
-                );
-                ecx.eq(goal.param_env, goal.predicate.term, assumption_projection_pred.term)
-                    .expect("expected goal term to be fully unconstrained");
-                ecx.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
+                then(ecx)
             })
         } else {
             Err(NoSolution)
diff --git a/compiler/rustc_trait_selection/src/solve/trait_goals.rs b/compiler/rustc_trait_selection/src/solve/trait_goals.rs
@@ -78,11 +78,11 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> {
         })
     }
 
-    fn consider_implied_clause(
+    fn probe_and_match_goal_against_assumption(
         ecx: &mut EvalCtxt<'_, 'tcx>,
         goal: Goal<'tcx, Self>,
         assumption: ty::Predicate<'tcx>,
-        requirements: impl IntoIterator<Item = Goal<'tcx, ty::Predicate<'tcx>>>,
+        then: impl FnOnce(&mut EvalCtxt<'_, 'tcx>) -> QueryResult<'tcx>,
     ) -> QueryResult<'tcx> {
         if let Some(poly_trait_pred) = assumption.to_opt_poly_trait_pred()
             && poly_trait_pred.def_id() == goal.predicate.def_id()
@@ -97,48 +97,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> {
                     goal.predicate.trait_ref,
                     assumption_trait_pred.trait_ref,
                 )?;
-                ecx.add_goals(requirements);
-                ecx.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
-            })
-        } else {
-            Err(NoSolution)
-        }
-    }
-
-    fn consider_object_bound_candidate(
-        ecx: &mut EvalCtxt<'_, 'tcx>,
-        goal: Goal<'tcx, Self>,
-        assumption: ty::Predicate<'tcx>,
-    ) -> QueryResult<'tcx> {
-        if let Some(poly_trait_pred) = assumption.to_opt_poly_trait_pred()
-            && poly_trait_pred.def_id() == goal.predicate.def_id()
-            && poly_trait_pred.polarity() == goal.predicate.polarity
-        {
-            // FIXME: Constness and polarity
-            ecx.probe(|ecx| {
-                let assumption_trait_pred =
-                    ecx.instantiate_binder_with_infer(poly_trait_pred);
-                ecx.eq(
-                    goal.param_env,
-                    goal.predicate.trait_ref,
-                    assumption_trait_pred.trait_ref,
-                )?;
-
-                let tcx = ecx.tcx();
-                let ty::Dynamic(bounds, _, _) = *goal.predicate.self_ty().kind() else {
-                    bug!("expected object type in `consider_object_bound_candidate`");
-                };
-                ecx.add_goals(
-                    structural_traits::predicates_for_object_candidate(
-                        &ecx,
-                        goal.param_env,
-                        goal.predicate.trait_ref,
-                        bounds,
-                    )
-                    .into_iter()
-                    .map(|pred| goal.with(tcx, pred)),
-                );
-                ecx.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
+                then(ecx)
             })
         } else {
             Err(NoSolution)
diff --git a/tests/ui/traits/new-solver/alias-bound-unsound.rs b/tests/ui/traits/new-solver/alias-bound-unsound.rs
@@ -0,0 +1,27 @@
+// compile-flags: -Ztrait-solver=next
+
+// Makes sure that alias bounds are not unsound!
+
+#![feature(trivial_bounds)]
+
+trait Foo {
+    type Item: Copy
+    where
+        <Self as Foo>::Item: Copy;
+
+    fn copy_me(x: &Self::Item) -> Self::Item {
+        *x
+    }
+}
+
+impl Foo for () {
+    type Item = String where String: Copy;
+}
+
+fn main() {
+    let x = String::from("hello, world");
+    drop(<() as Foo>::copy_me(&x));
+    //~^ ERROR `<() as Foo>::Item: Copy` is not satisfied
+    //~| ERROR `<() as Foo>::Item` is not well-formed
+    println!("{x}");
+}
diff --git a/tests/ui/traits/new-solver/alias-bound-unsound.stderr b/tests/ui/traits/new-solver/alias-bound-unsound.stderr
@@ -0,0 +1,24 @@
+error[E0277]: the trait bound `<() as Foo>::Item: Copy` is not satisfied
+  --> $DIR/alias-bound-unsound.rs:23:10
+   |
+LL |     drop(<() as Foo>::copy_me(&x));
+   |          ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Copy` is not implemented for `<() as Foo>::Item`
+   |
+note: required by a bound in `Foo::Item`
+  --> $DIR/alias-bound-unsound.rs:10:30
+   |
+LL |     type Item: Copy
+   |          ---- required by a bound in this associated type
+LL |     where
+LL |         <Self as Foo>::Item: Copy;
+   |                              ^^^^ required by this bound in `Foo::Item`
+
+error: the type `<() as Foo>::Item` is not well-formed
+  --> $DIR/alias-bound-unsound.rs:23:10
+   |
+LL |     drop(<() as Foo>::copy_me(&x));
+   |          ^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0277`.
diff --git a/tests/ui/traits/new-solver/nested-alias-bound.rs b/tests/ui/traits/new-solver/nested-alias-bound.rs
