diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index 4e5e756dc4af4..28a69a0f5ad45 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/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 (Vec, 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, ?t): Trait` + let trait_ref = header.subst(tcx, infer_substs); + + // `(Vec, ?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>, diff --git a/compiler/rustc_trait_selection/src/traits/mod.rs b/compiler/rustc_trait_selection/src/traits/mod.rs index 8d831dca6e309..365ff0b903062 100644 --- a/compiler/rustc_trait_selection/src/traits/mod.rs +++ b/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}; diff --git a/compiler/rustc_trait_selection/src/traits/specialize/specialization_graph.rs b/compiler/rustc_trait_selection/src/traits/specialize/specialization_graph.rs index aa5c624f471ff..4bcf4735feb97 100644 --- a/compiler/rustc_trait_selection/src/traits/specialize/specialization_graph.rs +++ b/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>), +enum OverlapResult<'tcx> { + /// The impl is going to be inserted as a new child in this group of children. + NoOverlap(Option>), /// The impl should replace existing impls [X1, ..], because the impl specializes X1, X2, etc. - ReplaceChildren(Vec), + SpecializeAll(Vec), /// 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, overlap_mode: OverlapMode, - ) -> Result, OverlapError<'tcx>>; + ) -> Result, 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, overlap_mode: OverlapMode, - ) -> Result, OverlapError<'tcx>> { - let mut last_lint = None; - let mut replace_children = Vec::new(); - + ) -> Result, 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>, + overlap_mode: OverlapMode, +) -> Result, 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 + '_ { @@ -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; } } diff --git a/tests/ui/coherence/coherence-overlap-negative-cycles.rs b/tests/ui/coherence/coherence-overlap-negative-cycles.rs new file mode 100644 index 0000000000000..f11a0fe7a1da1 --- /dev/null +++ b/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() {} diff --git a/tests/ui/coherence/coherence-overlap-negative-cycles.stderr b/tests/ui/coherence/coherence-overlap-negative-cycles.stderr new file mode 100644 index 0000000000000..672c6520dbf63 --- /dev/null +++ b/tests/ui/coherence/coherence-overlap-negative-cycles.stderr @@ -0,0 +1,12 @@ +error[E0119]: conflicting implementations of trait `MyTrait` for type `Foo` + --> $DIR/coherence-overlap-negative-cycles.rs:14:1 + | +LL | impl MyTrait for Foo where Foo: MyTrait {} + | ^^^^^^^^^^^^^^^^^^^^ + | | + | first implementation here + | conflicting implementation for `Foo` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0119`.