Skip to content

Commit cd805f0

Browse files
committed
Auto merge of #133830 - compiler-errors:span-key, r=lcnr
Rework dyn trait lowering to stop being so intertwined with trait alias expansion This PR reworks the trait object lowering code to stop handling trait aliases so funky, and removes the `TraitAliasExpander` in favor of a much simpler design. This refactoring is important for making the code that I'm writing in #133397 understandable and easy to maintain, so the diagnostics regressions are IMO inevitable. In the old trait object lowering code, we used to be a bit sloppy with the lists of traits in their unexpanded and expanded forms. This PR largely rewrites this logic to expand the trait aliases *once* and handle them more responsibly throughout afterwards. Please review this with whitespace disabled. r? lcnr
2 parents a7a6c64 + 824a867 commit cd805f0

26 files changed

+561
-642
lines changed

compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs

+85-118
Large diffs are not rendered by default.

compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs

+30-20
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,17 @@ use rustc_hir::def_id::DefId;
1111
use rustc_middle::bug;
1212
use rustc_middle::ty::print::{PrintPolyTraitRefExt as _, PrintTraitRefExt as _};
1313
use rustc_middle::ty::{
14-
self, AdtDef, Binder, GenericParamDefKind, TraitRef, Ty, TyCtxt, TypeVisitableExt,
14+
self, AdtDef, GenericParamDefKind, Ty, TyCtxt, TypeVisitableExt,
1515
suggest_constraining_type_param,
1616
};
1717
use rustc_session::parse::feature_err;
1818
use rustc_span::edit_distance::find_best_match_for_name;
1919
use rustc_span::{BytePos, DUMMY_SP, Ident, Span, Symbol, kw, sym};
2020
use rustc_trait_selection::error_reporting::traits::report_dyn_incompatibility;
2121
use rustc_trait_selection::traits::{
22-
FulfillmentError, TraitAliasExpansionInfo, dyn_compatibility_violations_for_assoc_item,
22+
FulfillmentError, dyn_compatibility_violations_for_assoc_item,
2323
};
24+
use smallvec::SmallVec;
2425

2526
use crate::errors::{
2627
self, AssocItemConstraintsNotAllowedHere, ManualImplementation, MissingTypeParams,
@@ -720,7 +721,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
720721
/// emit a generic note suggesting using a `where` clause to constraint instead.
721722
pub(crate) fn check_for_required_assoc_tys(
722723
&self,
723-
principal_span: Span,
724+
spans: SmallVec<[Span; 1]>,
724725
missing_assoc_types: FxIndexSet<(DefId, ty::PolyTraitRef<'tcx>)>,
725726
potential_assoc_types: Vec<usize>,
726727
trait_bounds: &[hir::PolyTraitRef<'_>],
@@ -729,6 +730,8 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
729730
return Ok(());
730731
}
731732

733+
let principal_span = *spans.first().unwrap();
734+
732735
let tcx = self.tcx();
733736
// FIXME: This logic needs some more care w.r.t handling of conflicts
734737
let missing_assoc_types: Vec<_> = missing_assoc_types
@@ -1124,29 +1127,36 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
11241127

11251128
pub fn report_trait_object_addition_traits_error(
11261129
&self,
1127-
regular_traits: &Vec<TraitAliasExpansionInfo<'_>>,
1130+
regular_traits: &Vec<(ty::PolyTraitPredicate<'tcx>, SmallVec<[Span; 1]>)>,
11281131
) -> ErrorGuaranteed {
1129-
let first_trait = &regular_traits[0];
1130-
let additional_trait = &regular_traits[1];
1132+
// we use the last span to point at the traits themselves,
1133+
// and all other preceding spans are trait alias expansions.
1134+
let (&first_span, first_alias_spans) = regular_traits[0].1.split_last().unwrap();
1135+
let (&second_span, second_alias_spans) = regular_traits[1].1.split_last().unwrap();
11311136
let mut err = struct_span_code_err!(
11321137
self.dcx(),
1133-
additional_trait.bottom().1,
1138+
*regular_traits[1].1.first().unwrap(),
11341139
E0225,
11351140
"only auto traits can be used as additional traits in a trait object"
11361141
);
1137-
additional_trait.label_with_exp_info(
1138-
&mut err,
1139-
"additional non-auto trait",
1140-
"additional use",
1141-
);
1142-
first_trait.label_with_exp_info(&mut err, "first non-auto trait", "first use");
1142+
err.span_label(first_span, "first non-auto trait");
1143+
for &alias_span in first_alias_spans {
1144+
err.span_label(alias_span, "first non-auto trait comes from this alias");
1145+
}
1146+
err.span_label(second_span, "additional non-auto trait");
1147+
for &alias_span in second_alias_spans {
1148+
err.span_label(alias_span, "second non-auto trait comes from this alias");
1149+
}
11431150
err.help(format!(
11441151
"consider creating a new trait with all of these as supertraits and using that \
11451152
trait here instead: `trait NewTrait: {} {{}}`",
11461153
regular_traits
11471154
.iter()
11481155
// FIXME: This should `print_sugared`, but also needs to integrate projection bounds...
1149-
.map(|t| t.trait_ref().print_only_trait_path().to_string())
1156+
.map(|(pred, _)| pred
1157+
.map_bound(|pred| pred.trait_ref)
1158+
.print_only_trait_path()
1159+
.to_string())
11501160
.collect::<Vec<_>>()
11511161
.join(" + "),
11521162
));
@@ -1161,14 +1171,14 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
11611171
pub fn report_trait_object_with_no_traits_error(
11621172
&self,
11631173
span: Span,
1164-
trait_bounds: &Vec<(Binder<'tcx, TraitRef<'tcx>>, Span)>,
1174+
user_written_clauses: impl IntoIterator<Item = (ty::Clause<'tcx>, Span)>,
11651175
) -> ErrorGuaranteed {
11661176
let tcx = self.tcx();
1167-
let trait_alias_span = trait_bounds
1168-
.iter()
1169-
.map(|&(trait_ref, _)| trait_ref.def_id())
1170-
.find(|&trait_ref| tcx.is_trait_alias(trait_ref))
1171-
.map(|trait_ref| tcx.def_span(trait_ref));
1177+
let trait_alias_span = user_written_clauses
1178+
.into_iter()
1179+
.filter_map(|(clause, _)| clause.as_trait_clause())
1180+
.find(|trait_ref| tcx.is_trait_alias(trait_ref.def_id()))
1181+
.map(|trait_ref| tcx.def_span(trait_ref.def_id()));
11721182

11731183
self.dcx().emit_err(TraitObjectDeclaredWithNoTraits { span, trait_alias_span })
11741184
}

compiler/rustc_hir_typeck/src/method/probe.rs

+5-6
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
use std::cell::{Cell, RefCell};
22
use std::cmp::max;
3-
use std::iter;
43
use std::ops::Deref;
54

65
use rustc_data_structures::fx::FxHashSet;
@@ -1009,11 +1008,11 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
10091008

10101009
if self.tcx.is_trait_alias(trait_def_id) {
10111010
// For trait aliases, recursively assume all explicitly named traits are relevant
1012-
for expansion in traits::expand_trait_aliases(
1013-
self.tcx,
1014-
iter::once((ty::Binder::dummy(trait_ref), self.span)),
1015-
) {
1016-
let bound_trait_ref = expansion.trait_ref();
1011+
for (bound_trait_pred, _) in
1012+
traits::expand_trait_aliases(self.tcx, [(trait_ref.upcast(self.tcx), self.span)]).0
1013+
{
1014+
assert_eq!(bound_trait_pred.polarity(), ty::PredicatePolarity::Positive);
1015+
let bound_trait_ref = bound_trait_pred.map_bound(|pred| pred.trait_ref);
10171016
for item in self.impl_or_trait_item(bound_trait_ref.def_id()) {
10181017
if !self.has_applicable_self(&item) {
10191018
self.record_static_candidate(CandidateSource::Trait(

compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,8 @@ pub fn report_dyn_incompatibility<'tcx>(
437437
tcx.dcx(),
438438
span,
439439
E0038,
440-
"the trait `{}` cannot be made into an object",
440+
"the {} `{}` cannot be made into an object",
441+
tcx.def_descr(trait_def_id),
441442
trait_str
442443
);
443444
err.span_label(span, format!("`{trait_str}` cannot be made into an object"));

compiler/rustc_trait_selection/src/traits/mod.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,9 @@ pub use self::specialize::{
6666
};
6767
pub use self::structural_normalize::StructurallyNormalizeExt;
6868
pub use self::util::{
69-
BoundVarReplacer, PlaceholderReplacer, TraitAliasExpander, TraitAliasExpansionInfo, elaborate,
70-
expand_trait_aliases, impl_item_is_final, supertraits,
71-
transitive_bounds_that_define_assoc_item, upcast_choices, with_replaced_escaping_bound_vars,
69+
BoundVarReplacer, PlaceholderReplacer, elaborate, expand_trait_aliases, impl_item_is_final,
70+
supertraits, transitive_bounds_that_define_assoc_item, upcast_choices,
71+
with_replaced_escaping_bound_vars,
7272
};
7373
use crate::error_reporting::InferCtxtErrorExt;
7474
use crate::infer::outlives::env::OutlivesEnvironment;

compiler/rustc_trait_selection/src/traits/util.rs

+66-143
Original file line numberDiff line numberDiff line change
@@ -1,162 +1,85 @@
1-
use std::collections::BTreeMap;
1+
use std::collections::{BTreeMap, VecDeque};
22

3-
use rustc_data_structures::fx::FxIndexMap;
4-
use rustc_errors::Diag;
3+
use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
54
use rustc_hir::def_id::DefId;
65
use rustc_infer::infer::InferCtxt;
76
pub use rustc_infer::traits::util::*;
87
use rustc_middle::bug;
98
use rustc_middle::ty::{
10-
self, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperFoldable, TypeVisitableExt, Upcast,
9+
self, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperFoldable, TypeVisitableExt,
1110
};
1211
use rustc_span::Span;
1312
use smallvec::{SmallVec, smallvec};
1413
use tracing::debug;
1514

16-
///////////////////////////////////////////////////////////////////////////
17-
// `TraitAliasExpander` iterator
18-
///////////////////////////////////////////////////////////////////////////
19-
20-
/// "Trait alias expansion" is the process of expanding a sequence of trait
21-
/// references into another sequence by transitively following all trait
22-
/// aliases. e.g. If you have bounds like `Foo + Send`, a trait alias
23-
/// `trait Foo = Bar + Sync;`, and another trait alias
24-
/// `trait Bar = Read + Write`, then the bounds would expand to
25-
/// `Read + Write + Sync + Send`.
26-
/// Expansion is done via a DFS (depth-first search), and the `visited` field
27-
/// is used to avoid cycles.
28-
pub struct TraitAliasExpander<'tcx> {
29-
tcx: TyCtxt<'tcx>,
30-
stack: Vec<TraitAliasExpansionInfo<'tcx>>,
31-
}
32-
33-
/// Stores information about the expansion of a trait via a path of zero or more trait aliases.
34-
#[derive(Debug, Clone)]
35-
pub struct TraitAliasExpansionInfo<'tcx> {
36-
pub path: SmallVec<[(ty::PolyTraitRef<'tcx>, Span); 4]>,
37-
}
38-
39-
impl<'tcx> TraitAliasExpansionInfo<'tcx> {
40-
fn new(trait_ref: ty::PolyTraitRef<'tcx>, span: Span) -> Self {
41-
Self { path: smallvec![(trait_ref, span)] }
42-
}
43-
44-
/// Adds diagnostic labels to `diag` for the expansion path of a trait through all intermediate
45-
/// trait aliases.
46-
pub fn label_with_exp_info(
47-
&self,
48-
diag: &mut Diag<'_>,
49-
top_label: &'static str,
50-
use_desc: &str,
51-
) {
52-
diag.span_label(self.top().1, top_label);
53-
if self.path.len() > 1 {
54-
for (_, sp) in self.path.iter().rev().skip(1).take(self.path.len() - 2) {
55-
diag.span_label(*sp, format!("referenced here ({use_desc})"));
56-
}
57-
}
58-
if self.top().1 != self.bottom().1 {
59-
// When the trait object is in a return type these two spans match, we don't want
60-
// redundant labels.
61-
diag.span_label(
62-
self.bottom().1,
63-
format!("trait alias used in trait object type ({use_desc})"),
64-
);
65-
}
66-
}
67-
68-
pub fn trait_ref(&self) -> ty::PolyTraitRef<'tcx> {
69-
self.top().0
70-
}
71-
72-
pub fn top(&self) -> &(ty::PolyTraitRef<'tcx>, Span) {
73-
self.path.last().unwrap()
74-
}
75-
76-
pub fn bottom(&self) -> &(ty::PolyTraitRef<'tcx>, Span) {
77-
self.path.first().unwrap()
78-
}
79-
80-
fn clone_and_push(&self, trait_ref: ty::PolyTraitRef<'tcx>, span: Span) -> Self {
81-
let mut path = self.path.clone();
82-
path.push((trait_ref, span));
83-
84-
Self { path }
85-
}
86-
}
87-
15+
/// Return the trait and projection predicates that come from eagerly expanding the
16+
/// trait aliases in the list of clauses. For each trait predicate, record a stack
17+
/// of spans that trace from the user-written trait alias bound. For projection predicates,
18+
/// just record the span of the projection itself.
19+
///
20+
/// For trait aliases, we don't deduplicte the predicates, since we currently do not
21+
/// consider duplicated traits as a single trait for the purposes of our "one trait principal"
22+
/// restriction; however, for projections we do deduplicate them.
23+
///
24+
/// ```rust,ignore (fails)
25+
/// trait Bar {}
26+
/// trait Foo = Bar + Bar;
27+
///
28+
/// let not_object_safe: dyn Foo; // bad, two `Bar` principals.
29+
/// ```
8830
pub fn expand_trait_aliases<'tcx>(
8931
tcx: TyCtxt<'tcx>,
90-
trait_refs: impl Iterator<Item = (ty::PolyTraitRef<'tcx>, Span)>,
91-
) -> TraitAliasExpander<'tcx> {
92-
let items: Vec<_> =
93-
trait_refs.map(|(trait_ref, span)| TraitAliasExpansionInfo::new(trait_ref, span)).collect();
94-
TraitAliasExpander { tcx, stack: items }
95-
}
96-
97-
impl<'tcx> TraitAliasExpander<'tcx> {
98-
/// If `item` is a trait alias and its predicate has not yet been visited, then expands `item`
99-
/// to the definition, pushes the resulting expansion onto `self.stack`, and returns `false`.
100-
/// Otherwise, immediately returns `true` if `item` is a regular trait, or `false` if it is a
101-
/// trait alias.
102-
/// The return value indicates whether `item` should be yielded to the user.
103-
fn expand(&mut self, item: &TraitAliasExpansionInfo<'tcx>) -> bool {
104-
let tcx = self.tcx;
105-
let trait_ref = item.trait_ref();
106-
let pred = trait_ref.upcast(tcx);
107-
108-
debug!("expand_trait_aliases: trait_ref={:?}", trait_ref);
109-
110-
// Don't recurse if this bound is not a trait alias.
111-
let is_alias = tcx.is_trait_alias(trait_ref.def_id());
112-
if !is_alias {
113-
return true;
114-
}
115-
116-
// Don't recurse if this trait alias is already on the stack for the DFS search.
117-
let anon_pred = anonymize_predicate(tcx, pred);
118-
if item
119-
.path
120-
.iter()
121-
.rev()
122-
.skip(1)
123-
.any(|&(tr, _)| anonymize_predicate(tcx, tr.upcast(tcx)) == anon_pred)
124-
{
125-
return false;
126-
}
127-
128-
// Get components of trait alias.
129-
let predicates = tcx.explicit_super_predicates_of(trait_ref.def_id());
130-
debug!(?predicates);
131-
132-
let items = predicates.skip_binder().iter().rev().filter_map(|(pred, span)| {
133-
pred.instantiate_supertrait(tcx, trait_ref)
134-
.as_trait_clause()
135-
.map(|trait_ref| item.clone_and_push(trait_ref.map_bound(|t| t.trait_ref), *span))
136-
});
137-
debug!("expand_trait_aliases: items={:?}", items.clone().collect::<Vec<_>>());
138-
139-
self.stack.extend(items);
140-
141-
false
142-
}
143-
}
144-
145-
impl<'tcx> Iterator for TraitAliasExpander<'tcx> {
146-
type Item = TraitAliasExpansionInfo<'tcx>;
147-
148-
fn size_hint(&self) -> (usize, Option<usize>) {
149-
(self.stack.len(), None)
150-
}
151-
152-
fn next(&mut self) -> Option<TraitAliasExpansionInfo<'tcx>> {
153-
while let Some(item) = self.stack.pop() {
154-
if self.expand(&item) {
155-
return Some(item);
32+
clauses: impl IntoIterator<Item = (ty::Clause<'tcx>, Span)>,
33+
) -> (
34+
Vec<(ty::PolyTraitPredicate<'tcx>, SmallVec<[Span; 1]>)>,
35+
Vec<(ty::PolyProjectionPredicate<'tcx>, Span)>,
36+
) {
37+
let mut trait_preds = vec![];
38+
let mut projection_preds = vec![];
39+
let mut seen_projection_preds = FxHashSet::default();
40+
41+
let mut queue: VecDeque<_> = clauses.into_iter().map(|(p, s)| (p, smallvec![s])).collect();
42+
43+
while let Some((clause, spans)) = queue.pop_front() {
44+
match clause.kind().skip_binder() {
45+
ty::ClauseKind::Trait(trait_pred) => {
46+
if tcx.is_trait_alias(trait_pred.def_id()) {
47+
queue.extend(
48+
tcx.explicit_super_predicates_of(trait_pred.def_id())
49+
.iter_identity_copied()
50+
.map(|(clause, span)| {
51+
let mut spans = spans.clone();
52+
spans.push(span);
53+
(
54+
clause.instantiate_supertrait(
55+
tcx,
56+
clause.kind().rebind(trait_pred.trait_ref),
57+
),
58+
spans,
59+
)
60+
}),
61+
);
62+
} else {
63+
trait_preds.push((clause.kind().rebind(trait_pred), spans));
64+
}
15665
}
66+
ty::ClauseKind::Projection(projection_pred) => {
67+
let projection_pred = clause.kind().rebind(projection_pred);
68+
if !seen_projection_preds.insert(tcx.anonymize_bound_vars(projection_pred)) {
69+
continue;
70+
}
71+
projection_preds.push((projection_pred, *spans.last().unwrap()));
72+
}
73+
ty::ClauseKind::RegionOutlives(..)
74+
| ty::ClauseKind::TypeOutlives(..)
75+
| ty::ClauseKind::ConstArgHasType(_, _)
76+
| ty::ClauseKind::WellFormed(_)
77+
| ty::ClauseKind::ConstEvaluatable(_)
78+
| ty::ClauseKind::HostEffect(..) => {}
15779
}
158-
None
15980
}
81+
82+
(trait_preds, projection_preds)
16083
}
16184

16285
///////////////////////////////////////////////////////////////////////////

0 commit comments

Comments
 (0)