Handle cycles in overlap with negative impls

compiler/rustc_trait_selection/src/traits/coherence.rs

@@ -120,6 +120,48 @@ pub fn overlapping_impls(
     Some(overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id, overlap_mode).unwrap())
 }
 
+/// Given an impl_def_id that "positively" implement a trait, check if the "negative" holds.
+pub fn negative_impl_holds(tcx: TyCtxt<'_>, impl_def_id: DefId, overlap_mode: OverlapMode) -> bool {
+    debug!("negative_impl_holds(impl1_header={:?}, overlap_mode={:?})", impl_def_id, overlap_mode);
+    // `for<T> (Vec<u32>, T): Trait`
+    let header = tcx.impl_trait_ref(impl_def_id).unwrap();
+
+    let infcx = tcx
+        .infer_ctxt()
+        .with_opaque_type_inference(DefiningAnchor::Bubble)
+        .intercrate(true)
+        .build();
+
+    // `[?t]`
+    let infer_substs = infcx.fresh_substs_for_item(DUMMY_SP, impl_def_id);
+
+    // `(Vec<u32>, ?t): Trait`
+    let trait_ref = header.subst(tcx, infer_substs);
+
+    // `(Vec<u32>, ?t): !Trait`
+    let trait_pred = tcx.mk_predicate(ty::Binder::dummy(ty::PredicateKind::Clause(
+        ty::Clause::Trait(ty::TraitPredicate {
+            trait_ref,
+            constness: ty::BoundConstness::NotConst,
+            polarity: ty::ImplPolarity::Negative,
+        }),
+    )));
+
+    // Ideally we would use param_env(impl_def_id) but that's unsound today.
+    let param_env = ty::ParamEnv::empty();
+
+    let selcx = &mut SelectionContext::new(&infcx);
+    selcx
+        .evaluate_root_obligation(&Obligation::new(
+            tcx,
+            ObligationCause::dummy(),
+            param_env,
+            trait_pred,
+        ))
+        .expect("Overflow should be caught earlier in standard query mode")
+        .must_apply_modulo_regions()
+}
+
 fn with_fresh_ty_vars<'cx, 'tcx>(
     selcx: &mut SelectionContext<'cx, 'tcx>,
     param_env: ty::ParamEnv<'tcx>,

compiler/rustc_trait_selection/src/traits/mod.rs

@@ -41,7 +41,9 @@ pub use self::ImplSource::*;
 pub use self::ObligationCauseCode::*;
 pub use self::SelectionError::*;
 
-pub use self::coherence::{add_placeholder_note, orphan_check, overlapping_impls};
+pub use self::coherence::{
+    add_placeholder_note, negative_impl_holds, orphan_check, overlapping_impls,
+};
 pub use self::coherence::{OrphanCheckErr, OverlapResult};
 pub use self::engine::{ObligationCtxt, TraitEngineExt};
 pub use self::fulfill::{FulfillmentContext, PendingPredicateObligation};

compiler/rustc_trait_selection/src/traits/specialize/specialization_graph.rs

@@ -20,17 +20,18 @@ pub struct FutureCompatOverlapError<'tcx> {
     pub kind: FutureCompatOverlapErrorKind,
 }
 
-/// The result of attempting to insert an impl into a group of children.
+/// The result of overlap_check, we would attempt to insert an impl into a group of children
+/// afterwards.
 #[derive(Debug)]
-enum Inserted<'tcx> {
-    /// The impl was inserted as a new child in this group of children.
-    BecameNewSibling(Option<FutureCompatOverlapError<'tcx>>),
+enum OverlapResult<'tcx> {
+    /// The impl is going to be inserted as a new child in this group of children.
+    NoOverlap(Option<FutureCompatOverlapError<'tcx>>),
 
     /// The impl should replace existing impls [X1, ..], because the impl specializes X1, X2, etc.
-    ReplaceChildren(Vec<DefId>),
+    SpecializeAll(Vec<DefId>),
 
     /// The impl is a specialization of an existing child.
-    ShouldRecurseOn(DefId),
+    SpecializeOne(DefId),
 }
 
 trait ChildrenExt<'tcx> {
@@ -43,7 +44,7 @@ trait ChildrenExt<'tcx> {
         impl_def_id: DefId,
         simplified_self: Option<SimplifiedType>,
         overlap_mode: OverlapMode,
-    ) -> Result<Inserted<'tcx>, OverlapError<'tcx>>;
+    ) -> Result<OverlapResult<'tcx>, OverlapError<'tcx>>;
 }
 
 impl<'tcx> ChildrenExt<'tcx> for Children {
@@ -90,123 +91,158 @@ impl<'tcx> ChildrenExt<'tcx> for Children {
         impl_def_id: DefId,
         simplified_self: Option<SimplifiedType>,
         overlap_mode: OverlapMode,
-    ) -> Result<Inserted<'tcx>, OverlapError<'tcx>> {
-        let mut last_lint = None;
-        let mut replace_children = Vec::new();
-
+    ) -> Result<OverlapResult<'tcx>, OverlapError<'tcx>> {
         let possible_siblings = match simplified_self {
             Some(st) => PotentialSiblings::Filtered(filtered_children(self, st)),
             None => PotentialSiblings::Unfiltered(iter_children(self)),
         };
 
-        for possible_sibling in possible_siblings {
-            debug!(?possible_sibling);
-
-            let create_overlap_error = |overlap: traits::coherence::OverlapResult<'tcx>| {
-                let trait_ref = overlap.impl_header.trait_ref.unwrap();
-                let self_ty = trait_ref.self_ty();
-
-                OverlapError {
-                    with_impl: possible_sibling,
-                    trait_ref,
-                    // Only report the `Self` type if it has at least
-                    // some outer concrete shell; otherwise, it's
-                    // not adding much information.
-                    self_ty: self_ty.has_concrete_skeleton().then_some(self_ty),
-                    intercrate_ambiguity_causes: overlap.intercrate_ambiguity_causes,
-                    involves_placeholder: overlap.involves_placeholder,
-                }
-            };
-
-            let report_overlap_error = |overlap: traits::coherence::OverlapResult<'tcx>,
-                                        last_lint: &mut _| {
-                // Found overlap, but no specialization; error out or report future-compat warning.
-
-                // Do we *still* get overlap if we disable the future-incompatible modes?
-                let should_err = traits::overlapping_impls(
-                    tcx,
-                    possible_sibling,
-                    impl_def_id,
-                    traits::SkipLeakCheck::default(),
-                    overlap_mode,
-                )
-                .is_some();
-
-                let error = create_overlap_error(overlap);
-
-                if should_err {
-                    Err(error)
-                } else {
-                    *last_lint = Some(FutureCompatOverlapError {
-                        error,
-                        kind: FutureCompatOverlapErrorKind::LeakCheck,
-                    });
-
-                    Ok((false, false))
-                }
-            };
+        let result = overlap_check_considering_specialization(
+            tcx,
+            impl_def_id,
+            possible_siblings,
+            overlap_mode,
+        );
+
+        if let Ok(OverlapResult::NoOverlap(_)) = result {
+            // No overlap with any potential siblings, so add as a new sibling.
+            debug!("placing as new sibling");
+            self.insert_blindly(tcx, impl_def_id);
+        }
+
+        result
+    }
+}
 
-            let last_lint_mut = &mut last_lint;
-            let (le, ge) = traits::overlapping_impls(
+fn overlap_check_considering_specialization<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    impl_def_id: DefId,
+    possible_siblings: PotentialSiblings<impl Iterator<Item = DefId>, impl Iterator<Item = DefId>>,
+    overlap_mode: OverlapMode,
+) -> Result<OverlapResult<'tcx>, OverlapError<'tcx>> {
+    let mut last_lint = None;
+    let mut replace_children = Vec::new();
+
+    for possible_sibling in possible_siblings {
+        debug!(?possible_sibling);
+
+        let create_overlap_error = |overlap: traits::coherence::OverlapResult<'tcx>| {
+            let trait_ref = overlap.impl_header.trait_ref.unwrap();
+            let self_ty = trait_ref.self_ty();
+
+            OverlapError {
+                with_impl: possible_sibling,
+                trait_ref,
+                // Only report the `Self` type if it has at least
+                // some outer concrete shell; otherwise, it's
+                // not adding much information.
+                self_ty: self_ty.has_concrete_skeleton().then_some(self_ty),
+                intercrate_ambiguity_causes: overlap.intercrate_ambiguity_causes,
+                involves_placeholder: overlap.involves_placeholder,
+            }
+        };
+
+        let report_overlap_error = |overlap: traits::coherence::OverlapResult<'tcx>,
+                                    last_lint: &mut _| {
+            // Found overlap, but no specialization; error out or report future-compat warning.
+
+            // Do we *still* get overlap if we disable the future-incompatible modes?
+            let should_err = traits::overlapping_impls(
                 tcx,
                 possible_sibling,
                 impl_def_id,
-                traits::SkipLeakCheck::Yes,
+                traits::SkipLeakCheck::default(),
                 overlap_mode,
             )
-            .map_or(Ok((false, false)), |overlap| {
-                if let Some(overlap_kind) =
-                    tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling)
-                {
-                    match overlap_kind {
-                        ty::ImplOverlapKind::Permitted { marker: _ } => {}
-                        ty::ImplOverlapKind::Issue33140 => {
-                            *last_lint_mut = Some(FutureCompatOverlapError {
-                                error: create_overlap_error(overlap),
-                                kind: FutureCompatOverlapErrorKind::Issue33140,
-                            });
-                        }
-                    }
+            .is_some();
+
+            let error = create_overlap_error(overlap);
+
+            if should_err {
+                Err(error)
+            } else {
+                *last_lint = Some(FutureCompatOverlapError {
+                    error,
+                    kind: FutureCompatOverlapErrorKind::LeakCheck,
+                });
 
-                    return Ok((false, false));
+                Ok((false, false))
+            }
+        };
+
+        let last_lint_mut = &mut last_lint;
+        let (le, ge) = traits::overlapping_impls(
+            tcx,
+            possible_sibling,
+            impl_def_id,
+            traits::SkipLeakCheck::Yes,
+            overlap_mode,
+        )
+        .map_or(Ok((false, false)), |overlap| {
+            if let Some(overlap_kind) =
+                tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling)
+            {
+                match overlap_kind {
+                    ty::ImplOverlapKind::Permitted { marker: _ } => {}
+                    ty::ImplOverlapKind::Issue33140 => {
+                        *last_lint_mut = Some(FutureCompatOverlapError {
+                            error: create_overlap_error(overlap),
+                            kind: FutureCompatOverlapErrorKind::Issue33140,
+                        });
+                    }
                 }
 
-                let le = tcx.specializes((impl_def_id, possible_sibling));
-                let ge = tcx.specializes((possible_sibling, impl_def_id));
+                return Ok((false, false));
+            }
 
-                if le == ge { report_overlap_error(overlap, last_lint_mut) } else { Ok((le, ge)) }
-            })?;
+            let le = tcx.specializes((impl_def_id, possible_sibling));
+            let ge = tcx.specializes((possible_sibling, impl_def_id));
 
-            if le && !ge {
-                debug!(
-                    "descending as child of TraitRef {:?}",
-                    tcx.impl_trait_ref(possible_sibling).unwrap().subst_identity()
-                );
+            if le == ge { report_overlap_error(overlap, last_lint_mut) } else { Ok((le, ge)) }
+        })?;
 
-                // The impl specializes `possible_sibling`.
-                return Ok(Inserted::ShouldRecurseOn(possible_sibling));
-            } else if ge && !le {
-                debug!(
-                    "placing as parent of TraitRef {:?}",
-                    tcx.impl_trait_ref(possible_sibling).unwrap().subst_identity()
-                );
+        if le && !ge {
+            debug!(
+                "descending as child of TraitRef {:?}",
+                tcx.impl_trait_ref(possible_sibling).unwrap().subst_identity()
+            );
 
-                replace_children.push(possible_sibling);
-            } else {
-                // Either there's no overlap, or the overlap was already reported by
-                // `overlap_error`.
-            }
-        }
+            // The impl specializes `possible_sibling`.
+            return Ok(OverlapResult::SpecializeOne(possible_sibling));
+        } else if ge && !le {
+            debug!(
+                "placing as parent of TraitRef {:?}",
+                tcx.impl_trait_ref(possible_sibling).unwrap().subst_identity()
+            );
 
-        if !replace_children.is_empty() {
-            return Ok(Inserted::ReplaceChildren(replace_children));
+            replace_children.push(possible_sibling);
+        } else {
+            // Either there's no overlap, or the overlap was already reported by
+            // `overlap_error`.
         }
+    }
 
-        // No overlap with any potential siblings, so add as a new sibling.
-        debug!("placing as new sibling");
-        self.insert_blindly(tcx, impl_def_id);
-        Ok(Inserted::BecameNewSibling(last_lint))
+    if !replace_children.is_empty() {
+        return Ok(OverlapResult::SpecializeAll(replace_children));
     }
+
+    if overlap_mode.use_negative_impl()
+        && tcx.impl_polarity(impl_def_id) == ty::ImplPolarity::Positive
+        && traits::negative_impl_holds(tcx, impl_def_id, overlap_mode)
+    {
+        let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap().skip_binder();
+        let self_ty = trait_ref.self_ty();
+
+        return Err(OverlapError {
+            with_impl: impl_def_id,
+            trait_ref,
+            self_ty: self_ty.has_concrete_skeleton().then_some(self_ty),
+            intercrate_ambiguity_causes: Default::default(),
+            involves_placeholder: false,
+        });
+    }
+
+    Ok(OverlapResult::NoOverlap(last_lint))
 }
 
 fn iter_children(children: &mut Children) -> impl Iterator<Item = DefId> + '_ {
@@ -306,7 +342,7 @@ impl<'tcx> GraphExt<'tcx> for Graph {
 
         // Descend the specialization tree, where `parent` is the current parent node.
         loop {
-            use self::Inserted::*;
+            use self::OverlapResult::*;
 
             let insert_result = self.children.entry(parent).or_default().insert(
                 tcx,
@@ -316,11 +352,11 @@ impl<'tcx> GraphExt<'tcx> for Graph {
             )?;
 
             match insert_result {
-                BecameNewSibling(opt_lint) => {
+                NoOverlap(opt_lint) => {
                     last_lint = opt_lint;
                     break;
                 }
-                ReplaceChildren(grand_children_to_be) => {
+                SpecializeAll(grand_children_to_be) => {
                     // We currently have
                     //
                     //     P
@@ -359,7 +395,7 @@ impl<'tcx> GraphExt<'tcx> for Graph {
                     }
                     break;
                 }
-                ShouldRecurseOn(new_parent) => {
+                SpecializeOne(new_parent) => {
                     parent = new_parent;
                 }
             }

tests/ui/coherence/coherence-overlap-negative-cycles.rs

@@ -0,0 +1,17 @@
+#![feature(trivial_bounds)]
+#![feature(negative_impls)]
+#![feature(rustc_attrs)]
+#![feature(with_negative_coherence)]
+#![allow(trivial_bounds)]
+
+#[rustc_strict_coherence]
+trait MyTrait {}
+
+struct Foo {}
+
+impl !MyTrait for Foo {}
+
+impl MyTrait for Foo where Foo: MyTrait {}
+//~^ ERROR: conflicting implementations of trait `MyTrait`
+
+fn main() {}