prioritize param env candidates if they don't guide type inference

compiler/rustc_middle/src/infer/canonical.rs

@@ -80,6 +80,18 @@ impl CanonicalVarValues<'_> {
             }
         })
     }
+
+    pub fn is_identity_modulo_regions(&self) -> bool {
+        self.var_values.iter().enumerate().all(|(bv, arg)| match arg.unpack() {
+            ty::GenericArgKind::Lifetime(_) => true,
+            ty::GenericArgKind::Type(ty) => {
+                matches!(*ty.kind(), ty::Bound(ty::INNERMOST, bt) if bt.var.as_usize() == bv)
+            }
+            ty::GenericArgKind::Const(ct) => {
+                matches!(ct.kind(), ty::ConstKind::Bound(ty::INNERMOST, bc) if bc.as_usize() == bv)
+            }
+        })
+    }
 }
 
 /// When we canonicalize a value to form a query, we wind up replacing

compiler/rustc_middle/src/traits/solve.rs

@@ -56,9 +56,19 @@ pub enum Certainty {
 impl Certainty {
     pub const AMBIGUOUS: Certainty = Certainty::Maybe(MaybeCause::Ambiguity);
 
-    /// When proving multiple goals using **AND**, e.g. nested obligations for an impl,
-    /// use this function to unify the certainty of these goals
-    pub fn unify_and(self, other: Certainty) -> Certainty {
+    /// Use this function to merge the certainty of multiple nested subgoals.
+    ///
+    /// Given an impl like `impl<T: Foo + Bar> Baz for T {}`, we have 2 nested
+    /// subgoals whenever we use the impl as a candidate: `T: Foo` and `T: Bar`.
+    /// If evaluating `T: Foo` results in ambiguity and `T: Bar` results in
+    /// success, we merge these two responses. This results in ambiguity.
+    ///
+    /// If we unify ambiguity with overflow, we return overflow. This doesn't matter
+    /// inside of the solver as we distinguish ambiguity from overflow. It does
+    /// however matter for diagnostics. If `T: Foo` resulted in overflow and `T: Bar`
+    /// in ambiguity without changing the inference state, we still want to tell the
+    /// user that `T: Baz` results in overflow.
+    pub fn unify_with(self, other: Certainty) -> Certainty {
         match (self, other) {
             (Certainty::Yes, Certainty::Yes) => Certainty::Yes,
             (Certainty::Yes, Certainty::Maybe(_)) => other,

compiler/rustc_trait_selection/src/solve/assembly.rs → compiler/rustc_trait_selection/src/solve/assembly/mod.rs

@@ -1,11 +1,9 @@
 //! Code shared by trait and projection goals for candidate assembly.
 
 use super::search_graph::OverflowHandler;
-#[cfg(doc)]
-use super::trait_goals::structural_traits::*;
 use super::{EvalCtxt, SolverMode};
+use crate::solve::CanonicalResponseExt;
 use crate::traits::coherence;
-use itertools::Itertools;
 use rustc_data_structures::fx::FxIndexSet;
 use rustc_hir::def_id::DefId;
 use rustc_infer::traits::query::NoSolution;
@@ -16,6 +14,8 @@ use rustc_middle::ty::TypeFoldable;
 use rustc_middle::ty::{self, Ty, TyCtxt};
 use std::fmt::Debug;
 
+pub(super) mod structural_traits;
+
 /// A candidate is a possible way to prove a goal.
 ///
 /// It consists of both the `source`, which describes how that goal would be proven,
@@ -547,61 +547,41 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
         }
     }
 
+    /// If there are multiple ways to prove a trait or projection goal, we have
+    /// to somehow try to merge the candidates into one. If that fails, we return
+    /// ambiguity.
     #[instrument(level = "debug", skip(self), ret)]
     pub(super) fn merge_candidates(
         &mut self,
         mut candidates: Vec<Candidate<'tcx>>,
     ) -> QueryResult<'tcx> {
-        match candidates.len() {
-            0 => return Err(NoSolution),
-            1 => return Ok(candidates.pop().unwrap().result),
-            _ => {}
+        // First try merging all candidates. This is complete and fully sound.
+        let responses = candidates.iter().map(|c| c.result).collect::<Vec<_>>();
+        if let Some(result) = self.try_merge_responses(&responses) {
+            return Ok(result);
         }
 
-        if candidates.len() > 1 {
-            let mut i = 0;
-            'outer: while i < candidates.len() {
-                for j in (0..candidates.len()).filter(|&j| i != j) {
-                    if self.candidate_should_be_dropped_in_favor_of(&candidates[i], &candidates[j])
-                    {
-                        debug!(candidate = ?candidates[i], "Dropping candidate #{}/{}", i, candidates.len());
-                        candidates.swap_remove(i);
-                        continue 'outer;
+        // We then check whether we should prioritize `ParamEnv` candidates.
+        //
+        // Doing so is incomplete and would therefore be unsound during coherence.
+        match self.solver_mode() {
+            SolverMode::Coherence => (),
+            // Prioritize `ParamEnv` candidates only if they do not guide inference.
+            //
+            // This is still incomplete as we may add incorrect region bounds.
+            SolverMode::Normal => {
+                let param_env_responses = candidates
+                    .iter()
+                    .filter(|c| matches!(c.source, CandidateSource::ParamEnv(_)))
+                    .map(|c| c.result)
+                    .collect::<Vec<_>>();
+                if let Some(result) = self.try_merge_responses(&param_env_responses) {
+                    if result.has_only_region_constraints() {
+                        return Ok(result);
                     }
                 }
-
-                debug!(candidate = ?candidates[i], "Retaining candidate #{}/{}", i, candidates.len());
-                i += 1;
             }
-
-            // If there are *STILL* multiple candidates that have *different* response
-            // results, give up and report ambiguity.
-            if candidates.len() > 1 && !candidates.iter().map(|cand| cand.result).all_equal() {
-                let certainty = if candidates.iter().all(|x| {
-                    matches!(x.result.value.certainty, Certainty::Maybe(MaybeCause::Overflow))
-                }) {
-                    Certainty::Maybe(MaybeCause::Overflow)
-                } else {
-                    Certainty::AMBIGUOUS
-                };
-                return self.evaluate_added_goals_and_make_canonical_response(certainty);
-            }
-        }
-
-        Ok(candidates.pop().unwrap().result)
-    }
-
-    fn candidate_should_be_dropped_in_favor_of(
-        &self,
-        candidate: &Candidate<'tcx>,
-        other: &Candidate<'tcx>,
-    ) -> bool {
-        // FIXME: implement this
-        match (candidate.source, other.source) {
-            (CandidateSource::Impl(_), _)
-            | (CandidateSource::ParamEnv(_), _)
-            | (CandidateSource::AliasBound, _)
-            | (CandidateSource::BuiltinImpl, _) => false,
         }
+        self.flounder(&responses)
     }
 }

compiler/rustc_trait_selection/src/solve/trait_goals/structural_traits.rs → compiler/rustc_trait_selection/src/solve/assembly/structural_traits.rs

@@ -11,7 +11,7 @@ use crate::solve::EvalCtxt;
 //
 // For types with an "existential" binder, i.e. generator witnesses, we also
 // instantiate the binder with placeholders eagerly.
-pub(super) fn instantiate_constituent_tys_for_auto_trait<'tcx>(
+pub(in crate::solve) fn instantiate_constituent_tys_for_auto_trait<'tcx>(
     ecx: &EvalCtxt<'_, 'tcx>,
     ty: Ty<'tcx>,
 ) -> Result<Vec<Ty<'tcx>>, NoSolution> {
@@ -87,7 +87,7 @@ pub(super) fn instantiate_constituent_tys_for_auto_trait<'tcx>(
     }
 }
 
-fn replace_erased_lifetimes_with_bound_vars<'tcx>(
+pub(in crate::solve) fn replace_erased_lifetimes_with_bound_vars<'tcx>(
     tcx: TyCtxt<'tcx>,
     ty: Ty<'tcx>,
 ) -> ty::Binder<'tcx, Ty<'tcx>> {
@@ -108,7 +108,7 @@ fn replace_erased_lifetimes_with_bound_vars<'tcx>(
     ty::Binder::bind_with_vars(ty, bound_vars)
 }
 
-pub(super) fn instantiate_constituent_tys_for_sized_trait<'tcx>(
+pub(in crate::solve) fn instantiate_constituent_tys_for_sized_trait<'tcx>(
     ecx: &EvalCtxt<'_, 'tcx>,
     ty: Ty<'tcx>,
 ) -> Result<Vec<Ty<'tcx>>, NoSolution> {
@@ -158,7 +158,7 @@ pub(super) fn instantiate_constituent_tys_for_sized_trait<'tcx>(
     }
 }
 
-pub(super) fn instantiate_constituent_tys_for_copy_clone_trait<'tcx>(
+pub(in crate::solve) fn instantiate_constituent_tys_for_copy_clone_trait<'tcx>(
     ecx: &EvalCtxt<'_, 'tcx>,
     ty: Ty<'tcx>,
 ) -> Result<Vec<Ty<'tcx>>, NoSolution> {
@@ -224,7 +224,7 @@ pub(super) fn instantiate_constituent_tys_for_copy_clone_trait<'tcx>(
 }
 
 // Returns a binder of the tupled inputs types and output type from a builtin callable type.
-pub(crate) fn extract_tupled_inputs_and_output_from_callable<'tcx>(
+pub(in crate::solve) fn extract_tupled_inputs_and_output_from_callable<'tcx>(
     tcx: TyCtxt<'tcx>,
     self_ty: Ty<'tcx>,
     goal_kind: ty::ClosureKind,
@@ -337,7 +337,13 @@ pub(crate) fn extract_tupled_inputs_and_output_from_callable<'tcx>(
 /// additional step of eagerly folding the associated types in the where
 /// clauses of the impl. In this example, that means replacing
 /// `<Self as Foo>::Bar` with `Ty` in the first impl.
-pub(crate) fn predicates_for_object_candidate<'tcx>(
+///
+// FIXME: This is only necessary as `<Self as Trait>::Assoc: ItemBound`
+// bounds in impls are trivially proven using the item bound candidates.
+// This is unsound in general and once that is fixed, we don't need to
+// normalize eagerly here. See https://github.com/lcnr/solver-woes/issues/9
+// for more details.
+pub(in crate::solve) fn predicates_for_object_candidate<'tcx>(
     ecx: &EvalCtxt<'_, 'tcx>,
     param_env: ty::ParamEnv<'tcx>,
     trait_ref: ty::TraitRef<'tcx>,

compiler/rustc_trait_selection/src/solve/eval_ctxt.rs

@@ -357,7 +357,7 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> {
                             // deal with `has_changed` in the next iteration.
                             new_goals.normalizes_to_hack_goal =
                                 Some(this.resolve_vars_if_possible(goal));
-                            has_changed = has_changed.map_err(|c| c.unify_and(certainty));
+                            has_changed = has_changed.map_err(|c| c.unify_with(certainty));
                         }
                     }
                 }
@@ -378,7 +378,7 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> {
                         Certainty::Yes => {}
                         Certainty::Maybe(_) => {
                             new_goals.goals.push(goal);
-                            has_changed = has_changed.map_err(|c| c.unify_and(certainty));
+                            has_changed = has_changed.map_err(|c| c.unify_with(certainty));
                         }
                     }
                 }

compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs

@@ -50,7 +50,7 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
         certainty: Certainty,
     ) -> QueryResult<'tcx> {
         let goals_certainty = self.try_evaluate_added_goals()?;
-        let certainty = certainty.unify_and(goals_certainty);
+        let certainty = certainty.unify_with(goals_certainty);
 
         let external_constraints = self.compute_external_query_constraints()?;
 

compiler/rustc_trait_selection/src/solve/mod.rs

@@ -46,6 +46,8 @@ enum SolverMode {
 
 trait CanonicalResponseExt {
     fn has_no_inference_or_external_constraints(&self) -> bool;
+
+    fn has_only_region_constraints(&self) -> bool;
 }
 
 impl<'tcx> CanonicalResponseExt for Canonical<'tcx, Response<'tcx>> {
@@ -54,6 +56,11 @@ impl<'tcx> CanonicalResponseExt for Canonical<'tcx, Response<'tcx>> {
             && self.value.var_values.is_identity()
             && self.value.external_constraints.opaque_types.is_empty()
     }
+
+    fn has_only_region_constraints(&self) -> bool {
+        self.value.var_values.is_identity_modulo_regions()
+            && self.value.external_constraints.opaque_types.is_empty()
+    }
 }
 
 impl<'a, 'tcx> EvalCtxt<'a, 'tcx> {
@@ -221,12 +228,17 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> {
             (Some(alias_lhs), Some(alias_rhs)) => {
                 debug!("both sides are aliases");
 
-                let candidates = vec![
-                    // LHS normalizes-to RHS
-                    evaluate_normalizes_to(self, alias_lhs, rhs, direction, Invert::No),
-                    // RHS normalizes-to RHS
-                    evaluate_normalizes_to(self, alias_rhs, lhs, direction, Invert::Yes),
-                    // Relate via substs
+                let mut candidates = Vec::new();
+                // LHS normalizes-to RHS
+                candidates.extend(
+                    evaluate_normalizes_to(self, alias_lhs, rhs, direction, Invert::No).ok(),
+                );
+                // RHS normalizes-to RHS
+                candidates.extend(
+                    evaluate_normalizes_to(self, alias_rhs, lhs, direction, Invert::Yes).ok(),
+                );
+                // Relate via substs
+                candidates.extend(
                     self.probe(|ecx| {
                         let span = tracing::span!(
                             tracing::Level::DEBUG,
@@ -247,11 +259,16 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> {
                         }
 
                         ecx.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
-                    }),
-                ];
+                    })
+                    .ok(),
+                );
                 debug!(?candidates);
 
-                self.try_merge_responses(candidates.into_iter())
+                if let Some(merged) = self.try_merge_responses(&candidates) {
+                    Ok(merged)
+                } else {
+                    self.flounder(&candidates)
+                }
             }
         }
     }
@@ -289,43 +306,51 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
         debug!("added_goals={:?}", &self.nested_goals.goals[current_len..]);
     }
 
-    #[instrument(level = "debug", skip(self, responses))]
+    /// Try to merge multiple possible ways to prove a goal, if that is not possible returns `None`.
+    ///
+    /// In this case we tend to flounder and return ambiguity by calling `[EvalCtxt::flounder]`.
+    #[instrument(level = "debug", skip(self), ret)]
     fn try_merge_responses(
         &mut self,
-        responses: impl Iterator<Item = QueryResult<'tcx>>,
-    ) -> QueryResult<'tcx> {
-        let candidates = responses.into_iter().flatten().collect::<Box<[_]>>();
-
-        if candidates.is_empty() {
-            return Err(NoSolution);
+        responses: &[CanonicalResponse<'tcx>],
+    ) -> Option<CanonicalResponse<'tcx>> {
+        if responses.is_empty() {
+            return None;
         }
 
         // FIXME(-Ztrait-solver=next): We should instead try to find a `Certainty::Yes` response with
         // a subset of the constraints that all the other responses have.
-        let one = candidates[0];
-        if candidates[1..].iter().all(|resp| resp == &one) {
-            return Ok(one);
+        let one = responses[0];
+        if responses[1..].iter().all(|&resp| resp == one) {
+            return Some(one);
         }
 
-        if let Some(response) = candidates.iter().find(|response| {
-            response.value.certainty == Certainty::Yes
-                && response.has_no_inference_or_external_constraints()
-        }) {
-            return Ok(*response);
-        }
+        responses
+            .iter()
+            .find(|response| {
+                response.value.certainty == Certainty::Yes
+                    && response.has_no_inference_or_external_constraints()
+            })
+            .copied()
+    }
 
-        let certainty = candidates.iter().fold(Certainty::AMBIGUOUS, |certainty, response| {
-            certainty.unify_and(response.value.certainty)
+    /// If we fail to merge responses we flounder and return overflow or ambiguity.
+    #[instrument(level = "debug", skip(self), ret)]
+    fn flounder(&mut self, responses: &[CanonicalResponse<'tcx>]) -> QueryResult<'tcx> {
+        if responses.is_empty() {
+            return Err(NoSolution);
+        }
+        let certainty = responses.iter().fold(Certainty::AMBIGUOUS, |certainty, response| {
+            certainty.unify_with(response.value.certainty)
         });
-        // FIXME(-Ztrait-solver=next): We should take the intersection of the constraints on all the
-        // responses and use that for the constraints of this ambiguous response.
-        debug!(">1 response, bailing with {certainty:?}");
+
         let response = self.evaluate_added_goals_and_make_canonical_response(certainty);
-        if let Ok(response) = &response {
+        if let Ok(response) = response {
             assert!(response.has_no_inference_or_external_constraints());
+            Ok(response)
+        } else {
+            bug!("failed to make floundered response: {responses:?}");
         }
-
-        response
     }
 }
 

compiler/rustc_trait_selection/src/solve/project_goals.rs

@@ -1,7 +1,6 @@
 use crate::traits::specialization_graph;
 
-use super::assembly;
-use super::trait_goals::structural_traits;
+use super::assembly::{self, structural_traits};
 use super::EvalCtxt;
 use rustc_errors::ErrorGuaranteed;
 use rustc_hir::def::DefKind;