Auto merge of #90104 - spastorino:coherence-for-negative-trait, r=nikomatsakis

Implement coherence checks for negative trait impls

The main purpose of this PR is to be able to [move Error trait to core](https://github.com/rust-lang/project-error-handling/issues/3).

This feature is necessary to handle the following from impl on box.

```rust
impl From<&str> for Box<dyn Error> { ... }
```

Without having negative traits affect coherence moving the error trait into `core` and moving that `From` impl to `alloc` will cause the from impl to no longer compiler because of a potential future incompatibility. The compiler indicates that `&str` _could_ introduce an `Error` impl in the future, and thus prevents the `From` impl in `alloc` that would cause overlap with `From<E: Error> for Box<dyn Error>`. Adding `impl !Error for &str {}` with the negative trait coherence feature will disable this error by encoding a stability guarantee that `&str` will never implement `Error`, making the `From` impl compile.

We would have this in `alloc`:

```rust
impl From<&str> for Box<dyn Error> {} // A
impl<E> From<E> for Box<dyn Error> where E: Error {} // B
```

and this in `core`:

```rust
trait Error {}
impl !Error for &str {}
```

r? `@nikomatsakis`

This PR was built on top of `@yaahc` PR #85764.

Language team proposal: to https://github.com/rust-lang/lang-team/issues/96

compiler/rustc_trait_selection/src/traits/auto_trait.rs

@@ -286,6 +286,8 @@ impl AutoTraitFinder<'tcx> {
                 substs: infcx.tcx.mk_substs_trait(ty, &[]),
             },
             constness: ty::BoundConstness::NotConst,
+            // Auto traits are positive
+            polarity: ty::ImplPolarity::Positive,
         }));
 
         let computed_preds = param_env.caller_bounds().iter();

compiler/rustc_trait_selection/src/traits/coherence.rs

@@ -5,9 +5,12 @@
 //! [trait-specialization]: https://rustc-dev-guide.rust-lang.org/traits/specialization.html
 
 use crate::infer::{CombinedSnapshot, InferOk, TyCtxtInferExt};
+use crate::traits::query::evaluate_obligation::InferCtxtExt;
 use crate::traits::select::IntercrateAmbiguityCause;
 use crate::traits::SkipLeakCheck;
-use crate::traits::{self, Normalized, Obligation, ObligationCause, SelectionContext};
+use crate::traits::{
+    self, Normalized, Obligation, ObligationCause, PredicateObligation, SelectionContext,
+};
 use rustc_hir::def_id::{DefId, LOCAL_CRATE};
 use rustc_middle::ty::fold::TypeFoldable;
 use rustc_middle::ty::subst::Subst;
@@ -158,6 +161,19 @@ fn overlap_within_probe(
     b_def_id: DefId,
     snapshot: &CombinedSnapshot<'_, 'tcx>,
 ) -> Option<OverlapResult<'tcx>> {
+    fn loose_check(selcx: &mut SelectionContext<'cx, 'tcx>, o: &PredicateObligation<'tcx>) -> bool {
+        !selcx.predicate_may_hold_fatal(o)
+    }
+
+    fn strict_check(selcx: &SelectionContext<'cx, 'tcx>, o: &PredicateObligation<'tcx>) -> bool {
+        let infcx = selcx.infcx();
+        let tcx = infcx.tcx;
+        o.flip_polarity(tcx)
+            .as_ref()
+            .map(|o| selcx.infcx().predicate_must_hold_modulo_regions(o))
+            .unwrap_or(false)
+    }
+
     // For the purposes of this check, we don't bring any placeholder
     // types into scope; instead, we replace the generic types with
     // fresh type variables, and hence we do our evaluations in an
@@ -184,8 +200,29 @@ fn overlap_within_probe(
 
     debug!("overlap: unification check succeeded");
 
-    // Are any of the obligations unsatisfiable? If so, no overlap.
+    // There's no overlap if obligations are unsatisfiable or if the obligation negated is
+    // satisfied.
+    //
+    // For example, given these two impl headers:
+    //
+    // `impl<'a> From<&'a str> for Box<dyn Error>`
+    // `impl<E> From<E> for Box<dyn Error> where E: Error`
+    //
+    // So we have:
+    //
+    // `Box<dyn Error>: From<&'?a str>`
+    // `Box<dyn Error>: From<?E>`
+    //
+    // After equating the two headers:
+    //
+    // `Box<dyn Error> = Box<dyn Error>`
+    // So, `?E = &'?a str` and then given the where clause `&'?a str: Error`.
+    //
+    // If the obligation `&'?a str: Error` holds, it means that there's overlap. If that doesn't
+    // hold we need to check if `&'?a str: !Error` holds, if doesn't hold there's overlap because
+    // at some point an impl for `&'?a str: Error` could be added.
     let infcx = selcx.infcx();
+    let tcx = infcx.tcx;
     let opt_failing_obligation = a_impl_header
         .predicates
         .iter()
@@ -199,7 +236,17 @@ fn overlap_within_probe(
             predicate: p,
         })
         .chain(obligations)
-        .find(|o| !selcx.predicate_may_hold_fatal(o));
+        .find(|o| {
+            // if both impl headers are set to strict coherence it means that this will be accepted
+            // only if it's stated that T: !Trait. So only prove that the negated obligation holds.
+            if tcx.has_attr(a_def_id, sym::rustc_strict_coherence)
+                && tcx.has_attr(b_def_id, sym::rustc_strict_coherence)
+            {
+                strict_check(selcx, o)
+            } else {
+                loose_check(selcx, o) || tcx.features().negative_impls && strict_check(selcx, o)
+            }
+        });
     // FIXME: the call to `selcx.predicate_may_hold_fatal` above should be ported
     // to the canonical trait query form, `infcx.predicate_may_hold`, once
     // the new system supports intercrate mode (which coherence needs).

compiler/rustc_trait_selection/src/traits/mod.rs

@@ -804,6 +804,7 @@ pub fn vtable_trait_upcasting_coercion_new_vptr_slot(
         ty::Binder::dummy(ty::TraitPredicate {
             trait_ref,
             constness: ty::BoundConstness::NotConst,
+            polarity: ty::ImplPolarity::Positive,
         }),
     );
 

compiler/rustc_trait_selection/src/traits/relationships.rs

@@ -44,6 +44,7 @@ pub(crate) fn update<'tcx, T>(
                             ty::PredicateKind::Trait(ty::TraitPredicate {
                                 trait_ref,
                                 constness: predicate.constness,
+                                polarity: predicate.polarity,
                             })
                         })
                         .to_predicate(infcx.tcx),

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

@@ -121,7 +121,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             return Ok(None);
         }
 
-        let mut candidates = candidate_set.vec;
+        let candidates = candidate_set.vec;
 
         debug!(?stack, ?candidates, "assembled {} candidates", candidates.len());
 
@@ -134,6 +134,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         // candidate which assumes $0 == int, one that assumes `$0 ==
         // usize`, etc. This spells an ambiguity.
 
+        let mut candidates = self.filter_impls(candidates, stack.obligation);
+
         // If there is more than one candidate, first winnow them down
         // by considering extra conditions (nested obligations and so
         // forth). We don't winnow if there is exactly one
@@ -149,7 +151,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         // Instead, we select the right impl now but report "`Bar` does
         // not implement `Clone`".
         if candidates.len() == 1 {
-            return self.filter_impls(candidates.pop().unwrap(), stack.obligation);
+            return self.filter_reservation_impls(candidates.pop().unwrap(), stack.obligation);
         }
 
         // Winnow, but record the exact outcome of evaluation, which
@@ -223,7 +225,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         }
 
         // Just one candidate left.
-        self.filter_impls(candidates.pop().unwrap().candidate, stack.obligation)
+        self.filter_reservation_impls(candidates.pop().unwrap().candidate, stack.obligation)
     }
 
     #[instrument(skip(self, stack), level = "debug")]
@@ -254,68 +256,75 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
 
         let mut candidates = SelectionCandidateSet { vec: Vec::new(), ambiguous: false };
 
-        self.assemble_candidates_for_trait_alias(obligation, &mut candidates);
-
-        // Other bounds. Consider both in-scope bounds from fn decl
-        // and applicable impls. There is a certain set of precedence rules here.
-        let def_id = obligation.predicate.def_id();
-        let lang_items = self.tcx().lang_items();
-
-        if lang_items.copy_trait() == Some(def_id) {
-            debug!(obligation_self_ty = ?obligation.predicate.skip_binder().self_ty());
-
-            // User-defined copy impls are permitted, but only for
-            // structs and enums.
+        // The only way to prove a NotImplemented(T: Foo) predicate is via a negative impl.
+        // There are no compiler built-in rules for this.
+        if obligation.polarity() == ty::ImplPolarity::Negative {
+            self.assemble_candidates_for_trait_alias(obligation, &mut candidates);
             self.assemble_candidates_from_impls(obligation, &mut candidates);
-
-            // For other types, we'll use the builtin rules.
-            let copy_conditions = self.copy_clone_conditions(obligation);
-            self.assemble_builtin_bound_candidates(copy_conditions, &mut candidates);
-        } else if lang_items.discriminant_kind_trait() == Some(def_id) {
-            // `DiscriminantKind` is automatically implemented for every type.
-            candidates.vec.push(DiscriminantKindCandidate);
-        } else if lang_items.pointee_trait() == Some(def_id) {
-            // `Pointee` is automatically implemented for every type.
-            candidates.vec.push(PointeeCandidate);
-        } else if lang_items.sized_trait() == Some(def_id) {
-            // Sized is never implementable by end-users, it is
-            // always automatically computed.
-            let sized_conditions = self.sized_conditions(obligation);
-            self.assemble_builtin_bound_candidates(sized_conditions, &mut candidates);
-        } else if lang_items.unsize_trait() == Some(def_id) {
-            self.assemble_candidates_for_unsizing(obligation, &mut candidates);
-        } else if lang_items.drop_trait() == Some(def_id)
-            && obligation.predicate.skip_binder().constness == ty::BoundConstness::ConstIfConst
-        {
-            if self.is_in_const_context {
-                self.assemble_const_drop_candidates(obligation, &mut candidates)?;
-            } else {
-                debug!("passing ~const Drop bound; in non-const context");
-                // `~const Drop` when we are not in a const context has no effect.
-                candidates.vec.push(ConstDropCandidate)
-            }
         } else {
-            if lang_items.clone_trait() == Some(def_id) {
-                // Same builtin conditions as `Copy`, i.e., every type which has builtin support
-                // for `Copy` also has builtin support for `Clone`, and tuples/arrays of `Clone`
-                // types have builtin support for `Clone`.
-                let clone_conditions = self.copy_clone_conditions(obligation);
-                self.assemble_builtin_bound_candidates(clone_conditions, &mut candidates);
+            self.assemble_candidates_for_trait_alias(obligation, &mut candidates);
+
+            // Other bounds. Consider both in-scope bounds from fn decl
+            // and applicable impls. There is a certain set of precedence rules here.
+            let def_id = obligation.predicate.def_id();
+            let lang_items = self.tcx().lang_items();
+
+            if lang_items.copy_trait() == Some(def_id) {
+                debug!(obligation_self_ty = ?obligation.predicate.skip_binder().self_ty());
+
+                // User-defined copy impls are permitted, but only for
+                // structs and enums.
+                self.assemble_candidates_from_impls(obligation, &mut candidates);
+
+                // For other types, we'll use the builtin rules.
+                let copy_conditions = self.copy_clone_conditions(obligation);
+                self.assemble_builtin_bound_candidates(copy_conditions, &mut candidates);
+            } else if lang_items.discriminant_kind_trait() == Some(def_id) {
+                // `DiscriminantKind` is automatically implemented for every type.
+                candidates.vec.push(DiscriminantKindCandidate);
+            } else if lang_items.pointee_trait() == Some(def_id) {
+                // `Pointee` is automatically implemented for every type.
+                candidates.vec.push(PointeeCandidate);
+            } else if lang_items.sized_trait() == Some(def_id) {
+                // Sized is never implementable by end-users, it is
+                // always automatically computed.
+                let sized_conditions = self.sized_conditions(obligation);
+                self.assemble_builtin_bound_candidates(sized_conditions, &mut candidates);
+            } else if lang_items.unsize_trait() == Some(def_id) {
+                self.assemble_candidates_for_unsizing(obligation, &mut candidates);
+            } else if lang_items.drop_trait() == Some(def_id)
+                && obligation.predicate.skip_binder().constness == ty::BoundConstness::ConstIfConst
+            {
+                if self.is_in_const_context {
+                    self.assemble_const_drop_candidates(obligation, &mut candidates)?;
+                } else {
+                    debug!("passing ~const Drop bound; in non-const context");
+                    // `~const Drop` when we are not in a const context has no effect.
+                    candidates.vec.push(ConstDropCandidate)
+                }
+            } else {
+                if lang_items.clone_trait() == Some(def_id) {
+                    // Same builtin conditions as `Copy`, i.e., every type which has builtin support
+                    // for `Copy` also has builtin support for `Clone`, and tuples/arrays of `Clone`
+                    // types have builtin support for `Clone`.
+                    let clone_conditions = self.copy_clone_conditions(obligation);
+                    self.assemble_builtin_bound_candidates(clone_conditions, &mut candidates);
+                }
+
+                self.assemble_generator_candidates(obligation, &mut candidates);
+                self.assemble_closure_candidates(obligation, &mut candidates);
+                self.assemble_fn_pointer_candidates(obligation, &mut candidates);
+                self.assemble_candidates_from_impls(obligation, &mut candidates);
+                self.assemble_candidates_from_object_ty(obligation, &mut candidates);
             }
 
-            self.assemble_generator_candidates(obligation, &mut candidates);
-            self.assemble_closure_candidates(obligation, &mut candidates);
-            self.assemble_fn_pointer_candidates(obligation, &mut candidates);
-            self.assemble_candidates_from_impls(obligation, &mut candidates);
-            self.assemble_candidates_from_object_ty(obligation, &mut candidates);
-        }
-
-        self.assemble_candidates_from_projected_tys(obligation, &mut candidates);
-        self.assemble_candidates_from_caller_bounds(stack, &mut candidates)?;
-        // Auto implementations have lower priority, so we only
-        // consider triggering a default if there is no other impl that can apply.
-        if candidates.vec.is_empty() {
-            self.assemble_candidates_from_auto_impls(obligation, &mut candidates);
+            self.assemble_candidates_from_projected_tys(obligation, &mut candidates);
+            self.assemble_candidates_from_caller_bounds(stack, &mut candidates)?;
+            // Auto implementations have lower priority, so we only
+            // consider triggering a default if there is no other impl that can apply.
+            if candidates.vec.is_empty() {
+                self.assemble_candidates_from_auto_impls(obligation, &mut candidates);
+            }
         }
         debug!("candidate list size: {}", candidates.vec.len());
         Ok(candidates)
@@ -376,7 +385,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         for bound in matching_bounds {
             let wc = self.evaluate_where_clause(stack, bound.value)?;
             if wc.may_apply() {
-                candidates.vec.push(ParamCandidate(bound));
+                candidates.vec.push(ParamCandidate((bound, stack.obligation.polarity())));
             }
         }
 
@@ -913,6 +922,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                         substs: self.tcx().mk_substs_trait(ty, &[]),
                     },
                     constness: ty::BoundConstness::NotConst,
+                    polarity: ty::ImplPolarity::Positive,
                 }));
             copy_obligation.recursion_depth = depth + 1;
             self.assemble_candidates_from_impls(&copy_obligation, &mut copy_candidates);

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

@@ -58,8 +58,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             }
 
             ParamCandidate(param) => {
-                let obligations = self.confirm_param_candidate(obligation, param.value);
-                Ok(ImplSource::Param(obligations, param.constness))
+                let obligations = self.confirm_param_candidate(obligation, param.0.value);
+                Ok(ImplSource::Param(obligations, param.0.constness))
             }
 
             ImplCandidate(impl_def_id) => {

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

@@ -20,7 +20,7 @@ use super::ObligationCauseCode;
 use super::Selection;
 use super::SelectionResult;
 use super::TraitQueryMode;
-use super::{ErrorReporting, Overflow, SelectionError, Unimplemented};
+use super::{ErrorReporting, Overflow, SelectionError};
 use super::{ObligationCause, PredicateObligation, TraitObligation};
 
 use crate::infer::{InferCtxt, InferOk, TypeFreshener};
@@ -709,7 +709,11 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
 
         debug!(?fresh_trait_ref);
 
-        if let Some(result) = self.check_evaluation_cache(obligation.param_env, fresh_trait_ref) {
+        if let Some(result) = self.check_evaluation_cache(
+            obligation.param_env,
+            fresh_trait_ref,
+            obligation.polarity(),
+        ) {
             debug!(?result, "CACHE HIT");
             return Ok(result);
         }
@@ -739,12 +743,19 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         let reached_depth = stack.reached_depth.get();
         if reached_depth >= stack.depth {
             debug!(?result, "CACHE MISS");
-            self.insert_evaluation_cache(obligation.param_env, fresh_trait_ref, dep_node, result);
+            self.insert_evaluation_cache(
+                obligation.param_env,
+                fresh_trait_ref,
+                obligation.polarity(),
+                dep_node,
+                result,
+            );
 
             stack.cache().on_completion(stack.dfn, |fresh_trait_ref, provisional_result| {
                 self.insert_evaluation_cache(
                     obligation.param_env,
                     fresh_trait_ref,
+                    obligation.polarity(),
                     dep_node,
                     provisional_result.max(result),
                 );
@@ -855,34 +866,39 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         // precise still.
         let unbound_input_types =
             stack.fresh_trait_ref.value.skip_binder().substs.types().any(|ty| ty.is_fresh());
-        // This check was an imperfect workaround for a bug in the old
-        // intercrate mode; it should be removed when that goes away.
-        if unbound_input_types && self.intercrate {
-            debug!("evaluate_stack --> unbound argument, intercrate -->  ambiguous",);
-            // Heuristics: show the diagnostics when there are no candidates in crate.
-            if self.intercrate_ambiguity_causes.is_some() {
-                debug!("evaluate_stack: intercrate_ambiguity_causes is some");
-                if let Ok(candidate_set) = self.assemble_candidates(stack) {
-                    if !candidate_set.ambiguous && candidate_set.vec.is_empty() {
-                        let trait_ref = stack.obligation.predicate.skip_binder().trait_ref;
-                        let self_ty = trait_ref.self_ty();
-                        let cause =
-                            with_no_trimmed_paths(|| IntercrateAmbiguityCause::DownstreamCrate {
-                                trait_desc: trait_ref.print_only_trait_path().to_string(),
-                                self_desc: if self_ty.has_concrete_skeleton() {
-                                    Some(self_ty.to_string())
-                                } else {
-                                    None
-                                },
+
+        if stack.obligation.polarity() != ty::ImplPolarity::Negative {
+            // This check was an imperfect workaround for a bug in the old
+            // intercrate mode; it should be removed when that goes away.
+            if unbound_input_types && self.intercrate {
+                debug!("evaluate_stack --> unbound argument, intercrate -->  ambiguous",);
+                // Heuristics: show the diagnostics when there are no candidates in crate.
+                if self.intercrate_ambiguity_causes.is_some() {
+                    debug!("evaluate_stack: intercrate_ambiguity_causes is some");
+                    if let Ok(candidate_set) = self.assemble_candidates(stack) {
+                        if !candidate_set.ambiguous && candidate_set.vec.is_empty() {
+                            let trait_ref = stack.obligation.predicate.skip_binder().trait_ref;
+                            let self_ty = trait_ref.self_ty();
+                            let cause = with_no_trimmed_paths(|| {
+                                IntercrateAmbiguityCause::DownstreamCrate {
+                                    trait_desc: trait_ref.print_only_trait_path().to_string(),
+                                    self_desc: if self_ty.has_concrete_skeleton() {
+                                        Some(self_ty.to_string())
+                                    } else {
+                                        None
+                                    },
+                                }
                             });
 
-                        debug!(?cause, "evaluate_stack: pushing cause");
-                        self.intercrate_ambiguity_causes.as_mut().unwrap().push(cause);
+                            debug!(?cause, "evaluate_stack: pushing cause");
+                            self.intercrate_ambiguity_causes.as_mut().unwrap().push(cause);
+                        }
                     }
                 }
+                return Ok(EvaluatedToAmbig);
             }
-            return Ok(EvaluatedToAmbig);
         }
+
         if unbound_input_types
             && stack.iter().skip(1).any(|prev| {
                 stack.obligation.param_env == prev.obligation.param_env
@@ -977,6 +993,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         &self,
         param_env: ty::ParamEnv<'tcx>,
         trait_ref: ty::ConstnessAnd<ty::PolyTraitRef<'tcx>>,
+        polarity: ty::ImplPolarity,
     ) -> Option<EvaluationResult> {
         // Neither the global nor local cache is aware of intercrate
         // mode, so don't do any caching. In particular, we might
@@ -988,17 +1005,19 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
 
         let tcx = self.tcx();
         if self.can_use_global_caches(param_env) {
-            if let Some(res) = tcx.evaluation_cache.get(&param_env.and(trait_ref), tcx) {
+            if let Some(res) = tcx.evaluation_cache.get(&(param_env.and(trait_ref), polarity), tcx)
+            {
                 return Some(res);
             }
         }
-        self.infcx.evaluation_cache.get(&param_env.and(trait_ref), tcx)
+        self.infcx.evaluation_cache.get(&(param_env.and(trait_ref), polarity), tcx)
     }
 
     fn insert_evaluation_cache(
         &mut self,
         param_env: ty::ParamEnv<'tcx>,
         trait_ref: ty::ConstnessAnd<ty::PolyTraitRef<'tcx>>,
+        polarity: ty::ImplPolarity,
         dep_node: DepNodeIndex,
         result: EvaluationResult,
     ) {
@@ -1023,13 +1042,17 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 // FIXME: Due to #50507 this overwrites the different values
                 // This should be changed to use HashMapExt::insert_same
                 // when that is fixed
-                self.tcx().evaluation_cache.insert(param_env.and(trait_ref), dep_node, result);
+                self.tcx().evaluation_cache.insert(
+                    (param_env.and(trait_ref), polarity),
+                    dep_node,
+                    result,
+                );
                 return;
             }
         }
 
         debug!(?trait_ref, ?result, "insert_evaluation_cache");
-        self.infcx.evaluation_cache.insert(param_env.and(trait_ref), dep_node, result);
+        self.infcx.evaluation_cache.insert((param_env.and(trait_ref), polarity), dep_node, result);
     }
 
     /// For various reasons, it's possible for a subobligation
@@ -1094,67 +1117,89 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         (result, dep_node)
     }
 
+    /// filter_impls filters constant trait obligations and candidates that have a positive impl
+    /// for a negative goal and a negative impl for a positive goal
     #[instrument(level = "debug", skip(self))]
     fn filter_impls(
+        &mut self,
+        candidates: Vec<SelectionCandidate<'tcx>>,
+        obligation: &TraitObligation<'tcx>,
+    ) -> Vec<SelectionCandidate<'tcx>> {
+        let tcx = self.tcx();
+        let mut result = Vec::with_capacity(candidates.len());
+
+        for candidate in candidates {
+            // Respect const trait obligations
+            if self.is_trait_predicate_const(obligation.predicate.skip_binder()) {
+                match candidate {
+                    // const impl
+                    ImplCandidate(def_id)
+                        if tcx.impl_constness(def_id) == hir::Constness::Const => {}
+                    // const param
+                    ParamCandidate((
+                        ty::ConstnessAnd { constness: ty::BoundConstness::ConstIfConst, .. },
+                        _,
+                    )) => {}
+                    // auto trait impl
+                    AutoImplCandidate(..) => {}
+                    // generator, this will raise error in other places
+                    // or ignore error with const_async_blocks feature
+                    GeneratorCandidate => {}
+                    // FnDef where the function is const
+                    FnPointerCandidate { is_const: true } => {}
+                    ConstDropCandidate => {}
+                    _ => {
+                        // reject all other types of candidates
+                        continue;
+                    }
+                }
+            }
+
+            if let ImplCandidate(def_id) = candidate {
+                if ty::ImplPolarity::Reservation == tcx.impl_polarity(def_id)
+                    || obligation.polarity() == tcx.impl_polarity(def_id)
+                    || self.allow_negative_impls
+                {
+                    result.push(candidate);
+                }
+            } else {
+                result.push(candidate);
+            }
+        }
+
+        result
+    }
+
+    /// filter_reservation_impls filter reservation impl for any goal as ambiguous
+    #[instrument(level = "debug", skip(self))]
+    fn filter_reservation_impls(
         &mut self,
         candidate: SelectionCandidate<'tcx>,
         obligation: &TraitObligation<'tcx>,
     ) -> SelectionResult<'tcx, SelectionCandidate<'tcx>> {
         let tcx = self.tcx();
-        // Respect const trait obligations
-        if self.is_trait_predicate_const(obligation.predicate.skip_binder()) {
-            match candidate {
-                // const impl
-                ImplCandidate(def_id) if tcx.impl_constness(def_id) == hir::Constness::Const => {}
-                // const param
-                ParamCandidate(ty::ConstnessAnd {
-                    constness: ty::BoundConstness::ConstIfConst,
-                    ..
-                }) => {}
-                // auto trait impl
-                AutoImplCandidate(..) => {}
-                // generator, this will raise error in other places
-                // or ignore error with const_async_blocks feature
-                GeneratorCandidate => {}
-                // FnDef where the function is const
-                FnPointerCandidate { is_const: true } => {}
-                ConstDropCandidate => {}
-                _ => {
-                    // reject all other types of candidates
-                    return Err(Unimplemented);
-                }
-            }
-        }
-        // Treat negative impls as unimplemented, and reservation impls as ambiguity.
+        // Treat reservation impls as ambiguity.
         if let ImplCandidate(def_id) = candidate {
-            match tcx.impl_polarity(def_id) {
-                ty::ImplPolarity::Negative if !self.allow_negative_impls => {
-                    return Err(Unimplemented);
-                }
-                ty::ImplPolarity::Reservation => {
-                    if let Some(intercrate_ambiguity_clauses) =
-                        &mut self.intercrate_ambiguity_causes
-                    {
-                        let attrs = tcx.get_attrs(def_id);
-                        let attr = tcx.sess.find_by_name(&attrs, sym::rustc_reservation_impl);
-                        let value = attr.and_then(|a| a.value_str());
-                        if let Some(value) = value {
-                            debug!(
-                                "filter_impls: \
+            if let ty::ImplPolarity::Reservation = tcx.impl_polarity(def_id) {
+                if let Some(intercrate_ambiguity_clauses) = &mut self.intercrate_ambiguity_causes {
+                    let attrs = tcx.get_attrs(def_id);
+                    let attr = tcx.sess.find_by_name(&attrs, sym::rustc_reservation_impl);
+                    let value = attr.and_then(|a| a.value_str());
+                    if let Some(value) = value {
+                        debug!(
+                            "filter_reservation_impls: \
                                  reservation impl ambiguity on {:?}",
-                                def_id
-                            );
-                            intercrate_ambiguity_clauses.push(
-                                IntercrateAmbiguityCause::ReservationImpl {
-                                    message: value.to_string(),
-                                },
-                            );
-                        }
+                            def_id
+                        );
+                        intercrate_ambiguity_clauses.push(
+                            IntercrateAmbiguityCause::ReservationImpl {
+                                message: value.to_string(),
+                            },
+                        );
                     }
-                    return Ok(None);
                 }
-                _ => {}
-            };
+                return Ok(None);
+            }
         }
         Ok(Some(candidate))
     }
@@ -1162,7 +1207,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     fn is_knowable<'o>(&mut self, stack: &TraitObligationStack<'o, 'tcx>) -> Option<Conflict> {
         debug!("is_knowable(intercrate={:?})", self.intercrate);
 
-        if !self.intercrate {
+        if !self.intercrate || stack.obligation.polarity() == ty::ImplPolarity::Negative {
             return None;
         }
 
@@ -1219,14 +1264,14 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         if self.can_use_global_caches(param_env) {
             if let Some(res) = tcx
                 .selection_cache
-                .get(&param_env.and(trait_ref).with_constness(pred.constness), tcx)
+                .get(&(param_env.and(trait_ref).with_constness(pred.constness), pred.polarity), tcx)
             {
                 return Some(res);
             }
         }
         self.infcx
             .selection_cache
-            .get(&param_env.and(trait_ref).with_constness(pred.constness), tcx)
+            .get(&(param_env.and(trait_ref).with_constness(pred.constness), pred.polarity), tcx)
     }
 
     /// Determines whether can we safely cache the result
@@ -1286,7 +1331,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                     debug!(?trait_ref, ?candidate, "insert_candidate_cache global");
                     // This may overwrite the cache with the same value.
                     tcx.selection_cache.insert(
-                        param_env.and(trait_ref).with_constness(pred.constness),
+                        (param_env.and(trait_ref).with_constness(pred.constness), pred.polarity),
                         dep_node,
                         candidate,
                     );
@@ -1297,7 +1342,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
 
         debug!(?trait_ref, ?candidate, "insert_candidate_cache local");
         self.infcx.selection_cache.insert(
-            param_env.and(trait_ref).with_constness(pred.constness),
+            (param_env.and(trait_ref).with_constness(pred.constness), pred.polarity),
             dep_node,
             candidate,
         );
@@ -1523,10 +1568,14 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 | ConstDropCandidate,
             ) => false,
 
-            (ParamCandidate(other), ParamCandidate(victim)) => {
+            (
+                ParamCandidate((other, other_polarity)),
+                ParamCandidate((victim, victim_polarity)),
+            ) => {
                 let same_except_bound_vars = other.value.skip_binder()
                     == victim.value.skip_binder()
                     && other.constness == victim.constness
+                    && other_polarity == victim_polarity
                     && !other.value.skip_binder().has_escaping_bound_vars();
                 if same_except_bound_vars {
                     // See issue #84398. In short, we can generate multiple ParamCandidates which are
@@ -1537,6 +1586,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                     other.value.bound_vars().len() <= victim.value.bound_vars().len()
                 } else if other.value == victim.value
                     && victim.constness == ty::BoundConstness::NotConst
+                    && other_polarity == victim_polarity
                 {
                     // Drop otherwise equivalent non-const candidates in favor of const candidates.
                     true
@@ -1566,11 +1616,11 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 | TraitAliasCandidate(..)
                 | ObjectCandidate(_)
                 | ProjectionCandidate(_),
-            ) => !is_global(&cand.value),
+            ) => !is_global(&cand.0.value),
             (ObjectCandidate(_) | ProjectionCandidate(_), ParamCandidate(ref cand)) => {
                 // Prefer these to a global where-clause bound
                 // (see issue #50825).
-                is_global(&cand.value)
+                is_global(&cand.0.value)
             }
             (
                 ImplCandidate(_)
@@ -1586,7 +1636,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             ) => {
                 // Prefer these to a global where-clause bound
                 // (see issue #50825).
-                is_global(&cand.value) && other.evaluation.must_apply_modulo_regions()
+                is_global(&cand.0.value) && other.evaluation.must_apply_modulo_regions()
             }
 
             (ProjectionCandidate(i), ProjectionCandidate(j))