Skip to content

Move overlap_mode into trait level attribute #93348

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions compiler/rustc_feature/src/active.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,9 @@ declare_features! (
(active, staged_api, "1.0.0", None, None),
/// Added for testing E0705; perma-unstable.
(active, test_2018_feature, "1.31.0", None, Some(Edition::Edition2018)),
/// Use for stable + negative coherence and strict coherence depending on trait's
/// rustc_strict_coherence value.
(active, with_negative_coherence, "1.60.0", None, None),
// !!!! !!!! !!!! !!!! !!!! !!!! !!!! !!!! !!!! !!!! !!!!
// Features are listed in alphabetical order. Tidy will fail if you don't keep it this way.
// !!!! !!!! !!!! !!!! !!!! !!!! !!!! !!!! !!!! !!!! !!!!
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,6 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
rustc_attr!(TEST, rustc_capture_analysis, Normal, template!(Word), WarnFollowing),
rustc_attr!(TEST, rustc_insignificant_dtor, Normal, template!(Word), WarnFollowing),
rustc_attr!(TEST, rustc_strict_coherence, Normal, template!(Word), WarnFollowing),
rustc_attr!(TEST, rustc_with_negative_coherence, Normal, template!(Word), WarnFollowing),
rustc_attr!(TEST, rustc_variance, Normal, template!(Word), WarnFollowing),
rustc_attr!(TEST, rustc_layout, Normal, template!(List: "field1, field2, ..."), WarnFollowing),
rustc_attr!(TEST, rustc_regions, Normal, template!(Word), WarnFollowing),
Expand Down
36 changes: 36 additions & 0 deletions compiler/rustc_middle/src/traits/specialization_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::ty::{self, TyCtxt};
use rustc_data_structures::fx::FxIndexMap;
use rustc_errors::ErrorReported;
use rustc_hir::def_id::{DefId, DefIdMap};
use rustc_span::symbol::sym;

/// A per-trait graph of impls in specialization order. At the moment, this
/// graph forms a tree rooted with the trait itself, with all other nodes
Expand Down Expand Up @@ -45,6 +46,41 @@ impl Graph {
}
}

/// What kind of overlap check are we doing -- this exists just for testing and feature-gating
/// purposes.
#[derive(Copy, Clone, PartialEq, Eq, Hash, HashStable, Debug, TyEncodable, TyDecodable)]
pub enum OverlapMode {
/// The 1.0 rules (either types fail to unify, or where clauses are not implemented for crate-local types)
Stable,
/// Feature-gated test: Stable, *or* there is an explicit negative impl that rules out one of the where-clauses.
WithNegative,
/// Just check for negative impls, not for "where clause not implemented": used for testing.
Strict,
}

impl OverlapMode {
pub fn get<'tcx>(tcx: TyCtxt<'tcx>, trait_id: DefId) -> OverlapMode {
let with_negative_coherence = tcx.features().with_negative_coherence;
let strict_coherence = tcx.has_attr(trait_id, sym::rustc_strict_coherence);

if with_negative_coherence {
if strict_coherence { OverlapMode::Strict } else { OverlapMode::WithNegative }
} else if strict_coherence {
bug!("To use strict_coherence you need to set with_negative_coherence feature flag");
} else {
OverlapMode::Stable
}
}

pub fn use_negative_impl(&self) -> bool {
*self == OverlapMode::Strict || *self == OverlapMode::WithNegative
}

pub fn use_implicit_negative(&self) -> bool {
*self == OverlapMode::Stable || *self == OverlapMode::WithNegative
}
}

/// Children of a given impl, grouped into blanket/non-blanket varieties as is
/// done in `TraitDef`.
#[derive(Default, TyEncodable, TyDecodable, Debug, HashStable)]
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1205,7 +1205,6 @@ symbols! {
rustc_trivial_field_reads,
rustc_unsafe_specialization_marker,
rustc_variance,
rustc_with_negative_coherence,
rustdoc,
rustdoc_internals,
rustfmt,
Expand Down Expand Up @@ -1489,6 +1488,7 @@ symbols! {
width,
windows,
windows_subsystem,
with_negative_coherence,
wrapping_add,
wrapping_mul,
wrapping_sub,
Expand Down
73 changes: 16 additions & 57 deletions compiler/rustc_trait_selection/src/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ use crate::traits::{
self, FulfillmentContext, Normalized, Obligation, ObligationCause, PredicateObligation,
PredicateObligations, SelectionContext,
};
use rustc_ast::Attribute;
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
use rustc_middle::traits::specialization_graph::OverlapMode;
use rustc_middle::ty::fast_reject::{self, SimplifyParams, StripReferences};
use rustc_middle::ty::fold::TypeFoldable;
use rustc_middle::ty::subst::Subst;
Expand Down Expand Up @@ -62,6 +62,7 @@ pub fn overlapping_impls<F1, F2, R>(
impl1_def_id: DefId,
impl2_def_id: DefId,
skip_leak_check: SkipLeakCheck,
overlap_mode: OverlapMode,
on_overlap: F1,
no_overlap: F2,
) -> R
Expand Down Expand Up @@ -99,7 +100,7 @@ where

let overlaps = tcx.infer_ctxt().enter(|infcx| {
let selcx = &mut SelectionContext::intercrate(&infcx);
overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id).is_some()
overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id, overlap_mode).is_some()
});

if !overlaps {
Expand All @@ -112,7 +113,9 @@ where
tcx.infer_ctxt().enter(|infcx| {
let selcx = &mut SelectionContext::intercrate(&infcx);
selcx.enable_tracking_intercrate_ambiguity_causes();
on_overlap(overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id).unwrap())
on_overlap(
overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id, overlap_mode).unwrap(),
)
})
}

Expand All @@ -138,68 +141,26 @@ fn with_fresh_ty_vars<'cx, 'tcx>(
header
}

/// What kind of overlap check are we doing -- this exists just for testing and feature-gating
/// purposes.
#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
enum OverlapMode {
/// The 1.0 rules (either types fail to unify, or where clauses are not implemented for crate-local types)
Stable,
/// Feature-gated test: Stable, *or* there is an explicit negative impl that rules out one of the where-clauses.
WithNegative,
/// Just check for negative impls, not for "where clause not implemented": used for testing.
Strict,
}

impl OverlapMode {
fn use_negative_impl(&self) -> bool {
*self == OverlapMode::Strict || *self == OverlapMode::WithNegative
}

fn use_implicit_negative(&self) -> bool {
*self == OverlapMode::Stable || *self == OverlapMode::WithNegative
}
}

fn overlap_mode<'tcx>(tcx: TyCtxt<'tcx>, impl1_def_id: DefId, impl2_def_id: DefId) -> OverlapMode {
// Find the possible coherence mode override opt-in attributes for each `DefId`
let find_coherence_attr = |attr: &Attribute| {
let name = attr.name_or_empty();
match name {
sym::rustc_with_negative_coherence | sym::rustc_strict_coherence => Some(name),
_ => None,
}
};
let impl1_coherence_mode = tcx.get_attrs(impl1_def_id).iter().find_map(find_coherence_attr);
let impl2_coherence_mode = tcx.get_attrs(impl2_def_id).iter().find_map(find_coherence_attr);

// If there are any (that currently happens in tests), they need to match. Otherwise, the
// default 1.0 rules are used.
match (impl1_coherence_mode, impl2_coherence_mode) {
(None, None) => OverlapMode::Stable,
(Some(sym::rustc_with_negative_coherence), Some(sym::rustc_with_negative_coherence)) => {
OverlapMode::WithNegative
}
(Some(sym::rustc_strict_coherence), Some(sym::rustc_strict_coherence)) => {
OverlapMode::Strict
}
(Some(mode), _) | (_, Some(mode)) => {
bug!("Use the same coherence mode on both impls: {}", mode)
}
}
}

/// Can both impl `a` and impl `b` be satisfied by a common type (including
/// where-clauses)? If so, returns an `ImplHeader` that unifies the two impls.
fn overlap<'cx, 'tcx>(
selcx: &mut SelectionContext<'cx, 'tcx>,
skip_leak_check: SkipLeakCheck,
impl1_def_id: DefId,
impl2_def_id: DefId,
overlap_mode: OverlapMode,
) -> Option<OverlapResult<'tcx>> {
debug!("overlap(impl1_def_id={:?}, impl2_def_id={:?})", impl1_def_id, impl2_def_id);

selcx.infcx().probe_maybe_skip_leak_check(skip_leak_check.is_yes(), |snapshot| {
overlap_within_probe(selcx, skip_leak_check, impl1_def_id, impl2_def_id, snapshot)
overlap_within_probe(
selcx,
skip_leak_check,
impl1_def_id,
impl2_def_id,
overlap_mode,
snapshot,
)
})
}

Expand All @@ -208,12 +169,10 @@ fn overlap_within_probe<'cx, 'tcx>(
skip_leak_check: SkipLeakCheck,
impl1_def_id: DefId,
impl2_def_id: DefId,
overlap_mode: OverlapMode,
snapshot: &CombinedSnapshot<'_, 'tcx>,
) -> Option<OverlapResult<'tcx>> {
let infcx = selcx.infcx();
let tcx = infcx.tcx;

let overlap_mode = overlap_mode(tcx, impl1_def_id, impl2_def_id);

if overlap_mode.use_negative_impl() {
if negative_impl(selcx, impl1_def_id, impl2_def_id)
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_trait_selection/src/traits/specialize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ pub(super) fn specialization_graph_provider(
trait_id: DefId,
) -> specialization_graph::Graph {
let mut sg = specialization_graph::Graph::new();
let overlap_mode = specialization_graph::OverlapMode::get(tcx, trait_id);

let mut trait_impls: Vec<_> = tcx.all_impls(trait_id).collect();

Expand All @@ -270,7 +271,7 @@ pub(super) fn specialization_graph_provider(
for impl_def_id in trait_impls {
if let Some(impl_def_id) = impl_def_id.as_local() {
// This is where impl overlap checking happens:
let insert_result = sg.insert(tcx, impl_def_id.to_def_id());
let insert_result = sg.insert(tcx, impl_def_id.to_def_id(), overlap_mode);
// Report error if there was one.
let (overlap, used_to_be_allowed) = match insert_result {
Err(overlap) => (Some(overlap), None),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ trait ChildrenExt<'tcx> {
tcx: TyCtxt<'tcx>,
impl_def_id: DefId,
simplified_self: Option<SimplifiedType>,
overlap_mode: OverlapMode,
) -> Result<Inserted, OverlapError>;
}

Expand Down Expand Up @@ -92,6 +93,7 @@ impl ChildrenExt<'_> for Children {
tcx: TyCtxt<'_>,
impl_def_id: DefId,
simplified_self: Option<SimplifiedType>,
overlap_mode: OverlapMode,
) -> Result<Inserted, OverlapError> {
let mut last_lint = None;
let mut replace_children = Vec::new();
Expand Down Expand Up @@ -142,6 +144,7 @@ impl ChildrenExt<'_> for Children {
possible_sibling,
impl_def_id,
traits::SkipLeakCheck::default(),
overlap_mode,
|_| true,
|| false,
);
Expand All @@ -166,6 +169,7 @@ impl ChildrenExt<'_> for Children {
possible_sibling,
impl_def_id,
traits::SkipLeakCheck::Yes,
overlap_mode,
|overlap| {
if let Some(overlap_kind) =
tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling)
Expand Down Expand Up @@ -273,6 +277,7 @@ pub trait GraphExt {
&mut self,
tcx: TyCtxt<'_>,
impl_def_id: DefId,
overlap_mode: OverlapMode,
) -> Result<Option<FutureCompatOverlapError>, OverlapError>;

/// Insert cached metadata mapping from a child impl back to its parent.
Expand All @@ -287,6 +292,7 @@ impl GraphExt for Graph {
&mut self,
tcx: TyCtxt<'_>,
impl_def_id: DefId,
overlap_mode: OverlapMode,
) -> Result<Option<FutureCompatOverlapError>, OverlapError> {
assert!(impl_def_id.is_local());

Expand Down Expand Up @@ -327,8 +333,12 @@ impl GraphExt for Graph {
loop {
use self::Inserted::*;

let insert_result =
self.children.entry(parent).or_default().insert(tcx, impl_def_id, simplified)?;
let insert_result = self.children.entry(parent).or_default().insert(
tcx,
impl_def_id,
simplified,
overlap_mode,
)?;

match insert_result {
BecameNewSibling(opt_lint) => {
Expand Down
13 changes: 12 additions & 1 deletion compiler/rustc_typeck/src/coherence/inherent_impls_overlap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_hir::itemlikevisit::ItemLikeVisitor;
use rustc_index::vec::IndexVec;
use rustc_middle::traits::specialization_graph::OverlapMode;
use rustc_middle::ty::{self, TyCtxt};
use rustc_span::Symbol;
use rustc_trait_selection::traits::{self, SkipLeakCheck};
Expand Down Expand Up @@ -99,14 +100,20 @@ impl<'tcx> InherentOverlapChecker<'tcx> {
}
}

fn check_for_overlapping_inherent_impls(&self, impl1_def_id: DefId, impl2_def_id: DefId) {
fn check_for_overlapping_inherent_impls(
&self,
overlap_mode: OverlapMode,
impl1_def_id: DefId,
impl2_def_id: DefId,
) {
traits::overlapping_impls(
self.tcx,
impl1_def_id,
impl2_def_id,
// We go ahead and just skip the leak check for
// inherent impls without warning.
SkipLeakCheck::Yes,
overlap_mode,
|overlap| {
self.check_for_common_items_in_impls(impl1_def_id, impl2_def_id, overlap);
false
Expand All @@ -131,6 +138,8 @@ impl<'tcx> ItemLikeVisitor<'_> for InherentOverlapChecker<'tcx> {
return;
}

let overlap_mode = OverlapMode::get(self.tcx, item.def_id.to_def_id());

let impls_items = impls
.iter()
.map(|impl_def_id| (impl_def_id, self.tcx.associated_items(*impl_def_id)))
Expand All @@ -145,6 +154,7 @@ impl<'tcx> ItemLikeVisitor<'_> for InherentOverlapChecker<'tcx> {
for &(&impl2_def_id, impl_items2) in &impls_items[(i + 1)..] {
if self.impls_have_common_items(impl_items1, impl_items2) {
self.check_for_overlapping_inherent_impls(
overlap_mode,
impl1_def_id,
impl2_def_id,
);
Expand Down Expand Up @@ -288,6 +298,7 @@ impl<'tcx> ItemLikeVisitor<'_> for InherentOverlapChecker<'tcx> {
let &(&impl2_def_id, impl_items2) = &impls_items[impl2_items_idx];
if self.impls_have_common_items(impl_items1, impl_items2) {
self.check_for_overlapping_inherent_impls(
overlap_mode,
impl1_def_id,
impl2_def_id,
);
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/coherence/auxiliary/option_future.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
#![crate_type = "lib"]
#![feature(negative_impls)]
#![feature(rustc_attrs)]
#![feature(with_negative_coherence)]

pub trait Future {}

#[rustc_with_negative_coherence]
impl<E> !Future for Option<E> where E: Sized {}
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
#![feature(negative_impls)]
#![feature(rustc_attrs)]
#![feature(trait_alias)]
#![feature(with_negative_coherence)]

trait A {}
trait B {}
trait AB = A + B;

impl !A for u32 {}

trait C {}
#[rustc_strict_coherence]
trait C {}
impl<T: AB> C for T {}
#[rustc_strict_coherence]
impl C for u32 {}
//~^ ERROR: conflicting implementations of trait `C` for type `u32` [E0119]
// FIXME this should work, we should implement an `assemble_neg_candidates` fn
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ error[E0119]: conflicting implementations of trait `C` for type `u32`
|
LL | impl<T: AB> C for T {}
| ------------------- first implementation here
LL | #[rustc_strict_coherence]
LL | impl C for u32 {}
| ^^^^^^^^^^^^^^ conflicting implementation for `u32`

Expand Down
Loading