Skip to content

Commit 498eeb7

Browse files
borsbors
committed
Auto merge of #93348 - spastorino:fix-perf-overlap-mode2, r=nikomatsakis
 Move overlap_mode into trait level attribute r? `@nikomatsakis` Should fix some performance regressions noted on #93175
2 parents 24b8bb1 + 0decf14 commit 498eeb7

17 files changed

+111
-75
lines changed

‎compiler/rustc_feature/src/active.rs

+3
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,9 @@ declare_features! (
161161
(active, staged_api, "1.0.0", None, None),
162162
/// Added for testing E0705; perma-unstable.
163163
(active, test_2018_feature, "1.31.0", None, Some(Edition::Edition2018)),
164+
/// Use for stable + negative coherence and strict coherence depending on trait's
165+
/// rustc_strict_coherence value.
166+
(active, with_negative_coherence, "1.60.0", None, None),
164167
// !!!! !!!! !!!! !!!! !!!! !!!! !!!! !!!! !!!! !!!! !!!!
165168
// Features are listed in alphabetical order. Tidy will fail if you don't keep it this way.
166169
// !!!! !!!! !!!! !!!! !!!! !!!! !!!! !!!! !!!! !!!! !!!!

‎compiler/rustc_feature/src/builtin_attrs.rs

-1
Original file line numberDiff line numberDiff line change
@@ -697,7 +697,6 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
697697
rustc_attr!(TEST, rustc_capture_analysis, Normal, template!(Word), WarnFollowing),
698698
rustc_attr!(TEST, rustc_insignificant_dtor, Normal, template!(Word), WarnFollowing),
699699
rustc_attr!(TEST, rustc_strict_coherence, Normal, template!(Word), WarnFollowing),
700-
rustc_attr!(TEST, rustc_with_negative_coherence, Normal, template!(Word), WarnFollowing),
701700
rustc_attr!(TEST, rustc_variance, Normal, template!(Word), WarnFollowing),
702701
rustc_attr!(TEST, rustc_layout, Normal, template!(List: "field1, field2, ..."), WarnFollowing),
703702
rustc_attr!(TEST, rustc_regions, Normal, template!(Word), WarnFollowing),

‎compiler/rustc_middle/src/traits/specialization_graph.rs

+36
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use crate::ty::{self, TyCtxt};
44
use rustc_data_structures::fx::FxIndexMap;
55
use rustc_errors::ErrorReported;
66
use rustc_hir::def_id::{DefId, DefIdMap};
7+
use rustc_span::symbol::sym;
78

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

49+
/// What kind of overlap check are we doing -- this exists just for testing and feature-gating
50+
/// purposes.
51+
#[derive(Copy, Clone, PartialEq, Eq, Hash, HashStable, Debug, TyEncodable, TyDecodable)]
52+
pub enum OverlapMode {
53+
/// The 1.0 rules (either types fail to unify, or where clauses are not implemented for crate-local types)
54+
Stable,
55+
/// Feature-gated test: Stable, *or* there is an explicit negative impl that rules out one of the where-clauses.
56+
WithNegative,
57+
/// Just check for negative impls, not for "where clause not implemented": used for testing.
58+
Strict,
59+
}
60+
61+
impl OverlapMode {
62+
pub fn get<'tcx>(tcx: TyCtxt<'tcx>, trait_id: DefId) -> OverlapMode {
63+
let with_negative_coherence = tcx.features().with_negative_coherence;
64+
let strict_coherence = tcx.has_attr(trait_id, sym::rustc_strict_coherence);
65+
66+
if with_negative_coherence {
67+
if strict_coherence { OverlapMode::Strict } else { OverlapMode::WithNegative }
68+
} else if strict_coherence {
69+
bug!("To use strict_coherence you need to set with_negative_coherence feature flag");
70+
} else {
71+
OverlapMode::Stable
72+
}
73+
}
74+
75+
pub fn use_negative_impl(&self) -> bool {
76+
*self == OverlapMode::Strict || *self == OverlapMode::WithNegative
77+
}
78+
79+
pub fn use_implicit_negative(&self) -> bool {
80+
*self == OverlapMode::Stable || *self == OverlapMode::WithNegative
81+
}
82+
}
83+
4884
/// Children of a given impl, grouped into blanket/non-blanket varieties as is
4985
/// done in `TraitDef`.
5086
#[derive(Default, TyEncodable, TyDecodable, Debug, HashStable)]

‎compiler/rustc_span/src/symbol.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1206,7 +1206,6 @@ symbols! {
12061206
rustc_trivial_field_reads,
12071207
rustc_unsafe_specialization_marker,
12081208
rustc_variance,
1209-
rustc_with_negative_coherence,
12101209
rustdoc,
12111210
rustdoc_internals,
12121211
rustfmt,
@@ -1490,6 +1489,7 @@ symbols! {
14901489
width,
14911490
windows,
14921491
windows_subsystem,
1492+
with_negative_coherence,
14931493
wrapping_add,
14941494
wrapping_mul,
14951495
wrapping_sub,

‎compiler/rustc_trait_selection/src/traits/coherence.rs

+16-57
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ use crate::traits::{
1313
self, FulfillmentContext, Normalized, Obligation, ObligationCause, PredicateObligation,
1414
PredicateObligations, SelectionContext,
1515
};
16-
use rustc_ast::Attribute;
1716
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
17+
use rustc_middle::traits::specialization_graph::OverlapMode;
1818
use rustc_middle::ty::fast_reject::{self, SimplifyParams, StripReferences};
1919
use rustc_middle::ty::fold::TypeFoldable;
2020
use rustc_middle::ty::subst::Subst;
@@ -62,6 +62,7 @@ pub fn overlapping_impls<F1, F2, R>(
6262
impl1_def_id: DefId,
6363
impl2_def_id: DefId,
6464
skip_leak_check: SkipLeakCheck,
65+
overlap_mode: OverlapMode,
6566
on_overlap: F1,
6667
no_overlap: F2,
6768
) -> R
@@ -99,7 +100,7 @@ where
99100

100101
let overlaps = tcx.infer_ctxt().enter(|infcx| {
101102
let selcx = &mut SelectionContext::intercrate(&infcx);
102-
overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id).is_some()
103+
overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id, overlap_mode).is_some()
103104
});
104105

105106
if !overlaps {
@@ -112,7 +113,9 @@ where
112113
tcx.infer_ctxt().enter(|infcx| {
113114
let selcx = &mut SelectionContext::intercrate(&infcx);
114115
selcx.enable_tracking_intercrate_ambiguity_causes();
115-
on_overlap(overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id).unwrap())
116+
on_overlap(
117+
overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id, overlap_mode).unwrap(),
118+
)
116119
})
117120
}
118121

@@ -138,68 +141,26 @@ fn with_fresh_ty_vars<'cx, 'tcx>(
138141
header
139142
}
140143

141-
/// What kind of overlap check are we doing -- this exists just for testing and feature-gating
142-
/// purposes.
143-
#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
144-
enum OverlapMode {
145-
/// The 1.0 rules (either types fail to unify, or where clauses are not implemented for crate-local types)
146-
Stable,
147-
/// Feature-gated test: Stable, *or* there is an explicit negative impl that rules out one of the where-clauses.
148-
WithNegative,
149-
/// Just check for negative impls, not for "where clause not implemented": used for testing.
150-
Strict,
151-
}
152-
153-
impl OverlapMode {
154-
fn use_negative_impl(&self) -> bool {
155-
*self == OverlapMode::Strict || *self == OverlapMode::WithNegative
156-
}
157-
158-
fn use_implicit_negative(&self) -> bool {
159-
*self == OverlapMode::Stable || *self == OverlapMode::WithNegative
160-
}
161-
}
162-
163-
fn overlap_mode<'tcx>(tcx: TyCtxt<'tcx>, impl1_def_id: DefId, impl2_def_id: DefId) -> OverlapMode {
164-
// Find the possible coherence mode override opt-in attributes for each `DefId`
165-
let find_coherence_attr = |attr: &Attribute| {
166-
let name = attr.name_or_empty();
167-
match name {
168-
sym::rustc_with_negative_coherence | sym::rustc_strict_coherence => Some(name),
169-
_ => None,
170-
}
171-
};
172-
let impl1_coherence_mode = tcx.get_attrs(impl1_def_id).iter().find_map(find_coherence_attr);
173-
let impl2_coherence_mode = tcx.get_attrs(impl2_def_id).iter().find_map(find_coherence_attr);
174-
175-
// If there are any (that currently happens in tests), they need to match. Otherwise, the
176-
// default 1.0 rules are used.
177-
match (impl1_coherence_mode, impl2_coherence_mode) {
178-
(None, None) => OverlapMode::Stable,
179-
(Some(sym::rustc_with_negative_coherence), Some(sym::rustc_with_negative_coherence)) => {
180-
OverlapMode::WithNegative
181-
}
182-
(Some(sym::rustc_strict_coherence), Some(sym::rustc_strict_coherence)) => {
183-
OverlapMode::Strict
184-
}
185-
(Some(mode), _) | (_, Some(mode)) => {
186-
bug!("Use the same coherence mode on both impls: {}", mode)
187-
}
188-
}
189-
}
190-
191144
/// Can both impl `a` and impl `b` be satisfied by a common type (including
192145
/// where-clauses)? If so, returns an `ImplHeader` that unifies the two impls.
193146
fn overlap<'cx, 'tcx>(
194147
selcx: &mut SelectionContext<'cx, 'tcx>,
195148
skip_leak_check: SkipLeakCheck,
196149
impl1_def_id: DefId,
197150
impl2_def_id: DefId,
151+
overlap_mode: OverlapMode,
198152
) -> Option<OverlapResult<'tcx>> {
199153
debug!("overlap(impl1_def_id={:?}, impl2_def_id={:?})", impl1_def_id, impl2_def_id);
200154

201155
selcx.infcx().probe_maybe_skip_leak_check(skip_leak_check.is_yes(), |snapshot| {
202-
overlap_within_probe(selcx, skip_leak_check, impl1_def_id, impl2_def_id, snapshot)
156+
overlap_within_probe(
157+
selcx,
158+
skip_leak_check,
159+
impl1_def_id,
160+
impl2_def_id,
161+
overlap_mode,
162+
snapshot,
163+
)
203164
})
204165
}
205166

@@ -208,12 +169,10 @@ fn overlap_within_probe<'cx, 'tcx>(
208169
skip_leak_check: SkipLeakCheck,
209170
impl1_def_id: DefId,
210171
impl2_def_id: DefId,
172+
overlap_mode: OverlapMode,
211173
snapshot: &CombinedSnapshot<'_, 'tcx>,
212174
) -> Option<OverlapResult<'tcx>> {
213175
let infcx = selcx.infcx();
214-
let tcx = infcx.tcx;
215-
216-
let overlap_mode = overlap_mode(tcx, impl1_def_id, impl2_def_id);
217176

218177
if overlap_mode.use_negative_impl() {
219178
if negative_impl(selcx, impl1_def_id, impl2_def_id)

‎compiler/rustc_trait_selection/src/traits/specialize/mod.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,7 @@ pub(super) fn specialization_graph_provider(
257257
trait_id: DefId,
258258
) -> specialization_graph::Graph {
259259
let mut sg = specialization_graph::Graph::new();
260+
let overlap_mode = specialization_graph::OverlapMode::get(tcx, trait_id);
260261

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

@@ -270,7 +271,7 @@ pub(super) fn specialization_graph_provider(
270271
for impl_def_id in trait_impls {
271272
if let Some(impl_def_id) = impl_def_id.as_local() {
272273
// This is where impl overlap checking happens:
273-
let insert_result = sg.insert(tcx, impl_def_id.to_def_id());
274+
let insert_result = sg.insert(tcx, impl_def_id.to_def_id(), overlap_mode);
274275
// Report error if there was one.
275276
let (overlap, used_to_be_allowed) = match insert_result {
276277
Err(overlap) => (Some(overlap), None),

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

+12-2
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ trait ChildrenExt<'tcx> {
4141
tcx: TyCtxt<'tcx>,
4242
impl_def_id: DefId,
4343
simplified_self: Option<SimplifiedType>,
44+
overlap_mode: OverlapMode,
4445
) -> Result<Inserted, OverlapError>;
4546
}
4647

@@ -92,6 +93,7 @@ impl ChildrenExt<'_> for Children {
9293
tcx: TyCtxt<'_>,
9394
impl_def_id: DefId,
9495
simplified_self: Option<SimplifiedType>,
96+
overlap_mode: OverlapMode,
9597
) -> Result<Inserted, OverlapError> {
9698
let mut last_lint = None;
9799
let mut replace_children = Vec::new();
@@ -142,6 +144,7 @@ impl ChildrenExt<'_> for Children {
142144
possible_sibling,
143145
impl_def_id,
144146
traits::SkipLeakCheck::default(),
147+
overlap_mode,
145148
|_| true,
146149
|| false,
147150
);
@@ -166,6 +169,7 @@ impl ChildrenExt<'_> for Children {
166169
possible_sibling,
167170
impl_def_id,
168171
traits::SkipLeakCheck::Yes,
172+
overlap_mode,
169173
|overlap| {
170174
if let Some(overlap_kind) =
171175
tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling)
@@ -273,6 +277,7 @@ pub trait GraphExt {
273277
&mut self,
274278
tcx: TyCtxt<'_>,
275279
impl_def_id: DefId,
280+
overlap_mode: OverlapMode,
276281
) -> Result<Option<FutureCompatOverlapError>, OverlapError>;
277282

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

@@ -327,8 +333,12 @@ impl GraphExt for Graph {
327333
loop {
328334
use self::Inserted::*;
329335

330-
let insert_result =
331-
self.children.entry(parent).or_default().insert(tcx, impl_def_id, simplified)?;
336+
let insert_result = self.children.entry(parent).or_default().insert(
337+
tcx,
338+
impl_def_id,
339+
simplified,
340+
overlap_mode,
341+
)?;
332342

333343
match insert_result {
334344
BecameNewSibling(opt_lint) => {

‎compiler/rustc_typeck/src/coherence/inherent_impls_overlap.rs

+12-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use rustc_hir as hir;
44
use rustc_hir::def_id::DefId;
55
use rustc_hir::itemlikevisit::ItemLikeVisitor;
66
use rustc_index::vec::IndexVec;
7+
use rustc_middle::traits::specialization_graph::OverlapMode;
78
use rustc_middle::ty::{self, TyCtxt};
89
use rustc_span::Symbol;
910
use rustc_trait_selection::traits::{self, SkipLeakCheck};
@@ -99,14 +100,20 @@ impl<'tcx> InherentOverlapChecker<'tcx> {
99100
}
100101
}
101102

102-
fn check_for_overlapping_inherent_impls(&self, impl1_def_id: DefId, impl2_def_id: DefId) {
103+
fn check_for_overlapping_inherent_impls(
104+
&self,
105+
overlap_mode: OverlapMode,
106+
impl1_def_id: DefId,
107+
impl2_def_id: DefId,
108+
) {
103109
traits::overlapping_impls(
104110
self.tcx,
105111
impl1_def_id,
106112
impl2_def_id,
107113
// We go ahead and just skip the leak check for
108114
// inherent impls without warning.
109115
SkipLeakCheck::Yes,
116+
overlap_mode,
110117
|overlap| {
111118
self.check_for_common_items_in_impls(impl1_def_id, impl2_def_id, overlap);
112119
false
@@ -131,6 +138,8 @@ impl<'tcx> ItemLikeVisitor<'_> for InherentOverlapChecker<'tcx> {
131138
return;
132139
}
133140

141+
let overlap_mode = OverlapMode::get(self.tcx, item.def_id.to_def_id());
142+
134143
let impls_items = impls
135144
.iter()
136145
.map(|impl_def_id| (impl_def_id, self.tcx.associated_items(*impl_def_id)))
@@ -145,6 +154,7 @@ impl<'tcx> ItemLikeVisitor<'_> for InherentOverlapChecker<'tcx> {
145154
for &(&impl2_def_id, impl_items2) in &impls_items[(i + 1)..] {
146155
if self.impls_have_common_items(impl_items1, impl_items2) {
147156
self.check_for_overlapping_inherent_impls(
157+
overlap_mode,
148158
impl1_def_id,
149159
impl2_def_id,
150160
);
@@ -288,6 +298,7 @@ impl<'tcx> ItemLikeVisitor<'_> for InherentOverlapChecker<'tcx> {
288298
let &(&impl2_def_id, impl_items2) = &impls_items[impl2_items_idx];
289299
if self.impls_have_common_items(impl_items1, impl_items2) {
290300
self.check_for_overlapping_inherent_impls(
301+
overlap_mode,
291302
impl1_def_id,
292303
impl2_def_id,
293304
);

‎src/test/ui/coherence/auxiliary/option_future.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
#![crate_type = "lib"]
22
#![feature(negative_impls)]
33
#![feature(rustc_attrs)]
4+
#![feature(with_negative_coherence)]
45

56
pub trait Future {}
67

7-
#[rustc_with_negative_coherence]
88
impl<E> !Future for Option<E> where E: Sized {}

‎src/test/ui/coherence/coherence-overlap-negate-alias-strict.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
11
#![feature(negative_impls)]
22
#![feature(rustc_attrs)]
33
#![feature(trait_alias)]
4+
#![feature(with_negative_coherence)]
45

56
trait A {}
67
trait B {}
78
trait AB = A + B;
89

910
impl !A for u32 {}
1011

11-
trait C {}
1212
#[rustc_strict_coherence]
13+
trait C {}
1314
impl<T: AB> C for T {}
14-
#[rustc_strict_coherence]
1515
impl C for u32 {}
1616
//~^ ERROR: conflicting implementations of trait `C` for type `u32` [E0119]
1717
// FIXME this should work, we should implement an `assemble_neg_candidates` fn

‎src/test/ui/coherence/coherence-overlap-negate-alias-strict.stderr

-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ error[E0119]: conflicting implementations of trait `C` for type `u32`
33
|
44
LL | impl<T: AB> C for T {}
55
| ------------------- first implementation here
6-
LL | #[rustc_strict_coherence]
76
LL | impl C for u32 {}
87
| ^^^^^^^^^^^^^^ conflicting implementation for `u32`
98

0 commit comments

Comments
 (0)