compare_impl_item: remove trivial bounds

compiler/rustc_hir_analysis/src/check/compare_impl_item.rs

@@ -12,7 +12,7 @@ use rustc_hir::{GenericParamKind, ImplItemKind};
 use rustc_infer::infer::outlives::env::OutlivesEnvironment;
 use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
 use rustc_infer::infer::{self, InferCtxt, TyCtxtInferExt};
-use rustc_infer::traits::util;
+use rustc_infer::traits::{util, Obligation};
 use rustc_middle::ty::error::{ExpectedFound, TypeError};
 use rustc_middle::ty::util::ExplicitSelf;
 use rustc_middle::ty::{
@@ -190,15 +190,13 @@ fn compare_method_predicate_entailment<'tcx>(
             .map(|(predicate, _)| predicate),
     );
 
+    let caller_bounds = filter_trivial_predicates(tcx, hybrid_preds.predicates);
+
     // Construct trait parameter environment and then shift it into the placeholder viewpoint.
     // The key step here is to update the caller_bounds's predicates to be
     // the new hybrid bounds we computed.
     let normalize_cause = traits::ObligationCause::misc(impl_m_span, impl_m_def_id);
-    let param_env = ty::ParamEnv::new(
-        tcx.mk_predicates(&hybrid_preds.predicates),
-        Reveal::UserFacing,
-        hir::Constness::NotConst,
-    );
+    let param_env = ty::ParamEnv::new(caller_bounds, Reveal::UserFacing, hir::Constness::NotConst);
     let param_env = traits::normalize_param_env_or_error(tcx, param_env, normalize_cause);
 
     let infcx = &tcx.infer_ctxt().build();
@@ -2080,3 +2078,83 @@ fn assoc_item_kind_str(impl_item: &ty::AssocItem) -> &'static str {
         ty::AssocKind::Type => "type",
     }
 }
+
+// FIXME(-Ztrait-solver=next): This hack should be unnecessary with the new trait
+// solver as it is better at dealing with ambiguity.
+//
+// Even if this code isn't completely trivial, it only removes predicates so it
+// should always remain sound.
+#[instrument(level = "debug", skip(tcx, predicates))]
+fn filter_trivial_predicates<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    mut predicates: Vec<ty::Predicate<'tcx>>,
+) -> &'tcx ty::List<ty::Predicate<'tcx>> {
+    // We start with a bad approximation of whether a predicate is trivial and put all
+    // non-trivial predicates into the environment used when checking whether the
+    // remaining ones are trivial.
+    let mut non_trivial_predicates = Vec::new();
+    for &predicate in predicates.iter() {
+        if !may_be_trivial_predicate(predicate) {
+            non_trivial_predicates.push(predicate);
+        }
+    }
+
+    let non_trivial_predicates = tcx.mk_predicates(&non_trivial_predicates);
+    if non_trivial_predicates.len() == predicates.len() {
+        non_trivial_predicates
+    } else {
+        let param_env =
+            ty::ParamEnv::new(non_trivial_predicates, Reveal::UserFacing, hir::Constness::NotConst);
+        predicates.retain(|&p| !is_trivial_predicate(tcx, param_env, p));
+        tcx.mk_predicates(&predicates)
+    }
+}
+
+// A bad approximation of whether a predicate is trivial. Used to put all non-trivial
+// predicates into the environment while checking whether the remaining ones are trivial.
+fn may_be_trivial_predicate<'tcx>(predicate: ty::Predicate<'tcx>) -> bool {
+    // We only consider trait and projection predicates which don't have a parameter
+    // as a self type as potentially non-trivial.
+    match predicate.kind().skip_binder() {
+        ty::PredicateKind::Clause(ty::Clause::Trait(predicate)) => {
+            !matches!(predicate.self_ty().kind(), ty::Param(_))
+        }
+        ty::PredicateKind::Clause(ty::Clause::Projection(predicate)) => {
+            !matches!(predicate.self_ty().kind(), ty::Param(_))
+        }
+        _ => false,
+    }
+}
+
+/// Returns whether `predicate` is trivially provable in the empty environment.
+///
+/// While it's definitely trivial if we return `Yes`,  this function is incomplete,
+/// so it may incorrectly return `No` even though the `predicate` is actually trivial.
+#[instrument(level = "debug", skip(tcx), ret)]
+fn is_trivial_predicate<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    param_env: ty::ParamEnv<'tcx>,
+    predicate: ty::Predicate<'tcx>,
+) -> bool {
+    if !may_be_trivial_predicate(predicate) {
+        return false;
+    }
+
+    let infcx = tcx.infer_ctxt().build();
+    // HACK: This can overflow and we must not abort here as that would break existing
+    // crates, most notably `typenum`.
+    //
+    // To deal with this we change overflow to only abort trait solving without
+    // aborting compilation. This means that this code isn't complete and may
+    // incorrectly error which is acceptable as this is just a best effort.
+    let ocx = ObligationCtxt::with_query_mode_canonical(&infcx);
+    let obligation = Obligation::new(tcx, ObligationCause::dummy(), param_env, predicate);
+    ocx.register_obligation(obligation);
+    if ocx.select_all_or_error().is_empty() {
+        let outlives_env = OutlivesEnvironment::new(param_env);
+        infcx.process_registered_region_obligations(outlives_env.region_bound_pairs(), param_env);
+        infcx.resolve_regions(&outlives_env).is_empty()
+    } else {
+        false
+    }
+}

compiler/rustc_trait_selection/src/traits/engine.rs

@@ -27,6 +27,13 @@ use rustc_span::Span;
 pub trait TraitEngineExt<'tcx> {
     fn new(tcx: TyCtxt<'tcx>) -> Box<Self>;
     fn new_in_snapshot(tcx: TyCtxt<'tcx>) -> Box<Self>;
+    /// Creates a new fulfillment context which does not abort compilation
+    /// on overflow.
+    ///
+    /// WARNING: Overflow will be returned as an error in `select_where_possible`
+    /// even though the overflow may not be the final result for the given obligation,
+    /// so you have to be incredibly careful when using `select_where_possible.
+    fn with_query_mode_canonical(tcx: TyCtxt<'tcx>) -> Box<Self>;
 }
 
 impl<'tcx> TraitEngineExt<'tcx> for dyn TraitEngine<'tcx> {
@@ -45,6 +52,14 @@ impl<'tcx> TraitEngineExt<'tcx> for dyn TraitEngine<'tcx> {
             TraitSolver::Next => Box::new(NextFulfillmentCtxt::new()),
         }
     }
+
+    fn with_query_mode_canonical(tcx: TyCtxt<'tcx>) -> Box<Self> {
+        match tcx.sess.opts.unstable_opts.trait_solver {
+            TraitSolver::Classic => Box::new(FulfillmentContext::with_query_mode_canonical()),
+            TraitSolver::Chalk => Box::new(ChalkFulfillmentContext::new()),
+            TraitSolver::Next => Box::new(NextFulfillmentCtxt::new()),
+        }
+    }
 }
 
 /// Used if you want to have pleasant experience when dealing
@@ -63,6 +78,18 @@ impl<'a, 'tcx> ObligationCtxt<'a, 'tcx> {
         Self { infcx, engine: RefCell::new(<dyn TraitEngine<'_>>::new_in_snapshot(infcx.tcx)) }
     }
 
+    /// You have to be incredibly careful when using this method.
+    ///
+    /// Used in places where we have to deal with overflow in a non-fatal way.
+    /// Note that by using this the trait solver ends up being incomplete in a
+    /// may way because overflow is returned as a hard error instead of ambiguity.
+    pub fn with_query_mode_canonical(infcx: &'a InferCtxt<'tcx>) -> Self {
+        Self {
+            infcx,
+            engine: RefCell::new(<dyn TraitEngine<'_>>::with_query_mode_canonical(infcx.tcx)),
+        }
+    }
+
     pub fn register_obligation(&self, obligation: PredicateObligation<'tcx>) {
         self.engine.borrow_mut().register_predicate_obligation(self.infcx, obligation);
     }

compiler/rustc_trait_selection/src/traits/fulfill.rs

@@ -12,7 +12,6 @@ use rustc_middle::ty::subst::SubstsRef;
 use rustc_middle::ty::{self, Binder, Const, TypeVisitableExt};
 use std::marker::PhantomData;
 
-use super::const_evaluatable;
 use super::project::{self, ProjectAndUnifyResult};
 use super::select::SelectionContext;
 use super::wf;
@@ -22,6 +21,7 @@ use super::CodeSelectionError;
 use super::EvaluationResult;
 use super::PredicateObligation;
 use super::Unimplemented;
+use super::{const_evaluatable, TraitQueryMode};
 use super::{FulfillmentError, FulfillmentErrorCode};
 
 use crate::traits::project::PolyProjectionObligation;
@@ -64,6 +64,8 @@ pub struct FulfillmentContext<'tcx> {
     // a snapshot (they don't *straddle* a snapshot, so there
     // is no trouble there).
     usable_in_snapshot: bool,
+
+    query_mode: TraitQueryMode,
 }
 
 #[derive(Clone, Debug)]
@@ -80,38 +82,30 @@ pub struct PendingPredicateObligation<'tcx> {
 #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
 static_assert_size!(PendingPredicateObligation<'_>, 72);
 
-impl<'a, 'tcx> FulfillmentContext<'tcx> {
+impl<'tcx> FulfillmentContext<'tcx> {
     /// Creates a new fulfillment context.
     pub(super) fn new() -> FulfillmentContext<'tcx> {
-        FulfillmentContext { predicates: ObligationForest::new(), usable_in_snapshot: false }
+        FulfillmentContext {
+            predicates: ObligationForest::new(),
+            usable_in_snapshot: false,
+            query_mode: TraitQueryMode::Standard,
+        }
     }
 
     pub(super) fn new_in_snapshot() -> FulfillmentContext<'tcx> {
-        FulfillmentContext { predicates: ObligationForest::new(), usable_in_snapshot: true }
+        FulfillmentContext {
+            predicates: ObligationForest::new(),
+            usable_in_snapshot: true,
+            query_mode: TraitQueryMode::Standard,
+        }
     }
 
-    /// Attempts to select obligations using `selcx`.
-    fn select(&mut self, selcx: SelectionContext<'a, 'tcx>) -> Vec<FulfillmentError<'tcx>> {
-        let span = debug_span!("select", obligation_forest_size = ?self.predicates.len());
-        let _enter = span.enter();
-
-        // Process pending obligations.
-        let outcome: Outcome<_, _> =
-            self.predicates.process_obligations(&mut FulfillProcessor { selcx });
-
-        // FIXME: if we kept the original cache key, we could mark projection
-        // obligations as complete for the projection cache here.
-
-        let errors: Vec<FulfillmentError<'tcx>> =
-            outcome.errors.into_iter().map(to_fulfillment_error).collect();
-
-        debug!(
-            "select({} predicates remaining, {} errors) done",
-            self.predicates.len(),
-            errors.len()
-        );
-
-        errors
+    pub(super) fn with_query_mode_canonical() -> FulfillmentContext<'tcx> {
+        FulfillmentContext {
+            predicates: ObligationForest::new(),
+            usable_in_snapshot: false,
+            query_mode: TraitQueryMode::Canonical,
+        }
     }
 }
 
@@ -138,8 +132,27 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentContext<'tcx> {
     }
 
     fn select_where_possible(&mut self, infcx: &InferCtxt<'tcx>) -> Vec<FulfillmentError<'tcx>> {
-        let selcx = SelectionContext::new(infcx);
-        self.select(selcx)
+        let span = debug_span!("select", obligation_forest_size = ?self.predicates.len());
+        let selcx = SelectionContext::with_query_mode(infcx, self.query_mode);
+        let _enter = span.enter();
+
+        // Process pending obligations.
+        let outcome: Outcome<_, _> =
+            self.predicates.process_obligations(&mut FulfillProcessor { selcx });
+
+        // FIXME: if we kept the original cache key, we could mark projection
+        // obligations as complete for the projection cache here.
+
+        let errors: Vec<FulfillmentError<'tcx>> =
+            outcome.errors.into_iter().map(to_fulfillment_error).collect();
+
+        debug!(
+            "select({} predicates remaining, {} errors) done",
+            self.predicates.len(),
+            errors.len()
+        );
+
+        errors
     }
 
     fn drain_unstalled_obligations(

tests/ui/implied-bounds/issue-108544-1.rs

@@ -0,0 +1,21 @@
+
+// check-pass
+
+use std::borrow::Cow;
+
+pub trait Trait {
+    fn method(self) -> Option<Cow<'static, str>>
+    where
+        Self: Sized;
+}
+
+impl<'a> Trait for Cow<'a, str> {
+    // We have to check `WF(return-type)` which requires `Cow<'static, str>: Sized`.
+    // If we use the `Self: Sized` bound from the trait method we end up equating
+    // `Cow<'a, str>` with `Cow<'static, str>`, causing an error.
+    fn method(self) -> Option<Cow<'static, str>> {
+        None
+    }
+}
+
+fn main() {}

tests/ui/implied-bounds/issue-108544-2.rs

@@ -0,0 +1,19 @@
+// check-pass
+
+// Similar to issue-108544.rs except that we have a generic `T` which
+// previously caused an overeager fast-path to trigger.
+use std::borrow::Cow;
+
+pub trait Trait<T: Clone> {
+    fn method(self) -> Option<Cow<'static, T>>
+    where
+        Self: Sized;
+}
+
+impl<'a, T: Clone> Trait<T> for Cow<'a, T> {
+    fn method(self) -> Option<Cow<'static, T>> {
+        None
+    }
+}
+
+fn main() {}

tests/ui/implied-bounds/trait-where-clause-implied.rs

@@ -0,0 +1,15 @@
+// check-pass
+
+pub trait Trait<'a, 'b> {
+    fn method(self, _: &'static &'static ())
+    where
+        'a: 'b;
+}
+
+impl<'a> Trait<'a, 'static> for () {
+    // On first glance, this seems like we have the extra implied bound that
+    // `'a: 'static`, but we know this from the trait method where clause.
+    fn method(self, _: &'static &'a ()) {}
+}
+
+fn main() {}