Skip to content

Commit

Permalink
Move overlap_mode into trait level attribute + feature flag
Browse files Browse the repository at this point in the history
  • Loading branch information
spastorino committed Jan 31, 2022
1 parent 427eba2 commit a9bfb5d
Show file tree
Hide file tree
Showing 17 changed files with 121 additions and 75 deletions.
3 changes: 3 additions & 0 deletions compiler/rustc_feature/src/active.rs
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
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
50 changes: 49 additions & 1 deletion compiler/rustc_middle/src/traits/specialization_graph.rs
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 @@ -31,11 +32,23 @@ pub struct Graph {

/// Whether an error was emitted while constructing the graph.
pub has_errored: bool,

/// Overlap mode to be used
pub overlap_mode: OverlapMode,
}

impl Graph {
pub fn new() -> Graph {
Graph { parent: Default::default(), children: Default::default(), has_errored: false }
Graph {
parent: Default::default(),
children: Default::default(),
has_errored: false,
overlap_mode: OverlapMode::Stable,
}
}

pub fn set_overlap_mode(&mut self, overlap_mode: OverlapMode) {
self.overlap_mode = overlap_mode;
}

/// The parent of a given impl, which is the `DefId` of the trait when the
Expand All @@ -45,6 +58,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
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
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
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();
sg.set_overlap_mode(specialization_graph::OverlapMode::get(tcx, trait_id));

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

Expand Down
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 @@ -327,8 +331,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,
self.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
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
@@ -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 {}
@@ -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
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

0 comments on commit a9bfb5d

Please sign in to comment.