Auto merge of rust-lang#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 rust-lang#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

Author: bors

compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs

@@ -11,16 +11,17 @@ use rustc_hir::def_id::DefId;
 use rustc_middle::bug;
 use rustc_middle::ty::print::{PrintPolyTraitRefExt as _, PrintTraitRefExt as _};
 use rustc_middle::ty::{
-    self, AdtDef, Binder, GenericParamDefKind, TraitRef, Ty, TyCtxt, TypeVisitableExt,
+    self, AdtDef, GenericParamDefKind, Ty, TyCtxt, TypeVisitableExt,
     suggest_constraining_type_param,
 };
 use rustc_session::parse::feature_err;
 use rustc_span::edit_distance::find_best_match_for_name;
 use rustc_span::{BytePos, DUMMY_SP, Ident, Span, Symbol, kw, sym};
 use rustc_trait_selection::error_reporting::traits::report_dyn_incompatibility;
 use rustc_trait_selection::traits::{
-    FulfillmentError, TraitAliasExpansionInfo, dyn_compatibility_violations_for_assoc_item,
+    FulfillmentError, dyn_compatibility_violations_for_assoc_item,
 };
+use smallvec::SmallVec;
 
 use crate::errors::{
     self, AssocItemConstraintsNotAllowedHere, ManualImplementation, MissingTypeParams,
@@ -720,7 +721,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
     /// emit a generic note suggesting using a `where` clause to constraint instead.
     pub(crate) fn check_for_required_assoc_tys(
         &self,
-        principal_span: Span,
+        spans: SmallVec<[Span; 1]>,
         missing_assoc_types: FxIndexSet<(DefId, ty::PolyTraitRef<'tcx>)>,
         potential_assoc_types: Vec<usize>,
         trait_bounds: &[hir::PolyTraitRef<'_>],
@@ -729,6 +730,8 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
             return Ok(());
         }
 
+        let principal_span = *spans.first().unwrap();
+
         let tcx = self.tcx();
         // FIXME: This logic needs some more care w.r.t handling of conflicts
         let missing_assoc_types: Vec<_> = missing_assoc_types
@@ -1124,29 +1127,36 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
 
     pub fn report_trait_object_addition_traits_error(
         &self,
-        regular_traits: &Vec<TraitAliasExpansionInfo<'_>>,
+        regular_traits: &Vec<(ty::PolyTraitPredicate<'tcx>, SmallVec<[Span; 1]>)>,
     ) -> ErrorGuaranteed {
-        let first_trait = &regular_traits[0];
-        let additional_trait = &regular_traits[1];
+        // we use the last span to point at the traits themselves,
+        // and all other preceding spans are trait alias expansions.
+        let (&first_span, first_alias_spans) = regular_traits[0].1.split_last().unwrap();
+        let (&second_span, second_alias_spans) = regular_traits[1].1.split_last().unwrap();
         let mut err = struct_span_code_err!(
             self.dcx(),
-            additional_trait.bottom().1,
+            *regular_traits[1].1.first().unwrap(),
             E0225,
             "only auto traits can be used as additional traits in a trait object"
         );
-        additional_trait.label_with_exp_info(
-            &mut err,
-            "additional non-auto trait",
-            "additional use",
-        );
-        first_trait.label_with_exp_info(&mut err, "first non-auto trait", "first use");
+        err.span_label(first_span, "first non-auto trait");
+        for &alias_span in first_alias_spans {
+            err.span_label(alias_span, "first non-auto trait comes from this alias");
+        }
+        err.span_label(second_span, "additional non-auto trait");
+        for &alias_span in second_alias_spans {
+            err.span_label(alias_span, "second non-auto trait comes from this alias");
+        }
         err.help(format!(
             "consider creating a new trait with all of these as supertraits and using that \
              trait here instead: `trait NewTrait: {} {{}}`",
             regular_traits
                 .iter()
                 // FIXME: This should `print_sugared`, but also needs to integrate projection bounds...
-                .map(|t| t.trait_ref().print_only_trait_path().to_string())
+                .map(|(pred, _)| pred
+                    .map_bound(|pred| pred.trait_ref)
+                    .print_only_trait_path()
+                    .to_string())
                 .collect::<Vec<_>>()
                 .join(" + "),
         ));
@@ -1161,14 +1171,14 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
     pub fn report_trait_object_with_no_traits_error(
         &self,
         span: Span,
-        trait_bounds: &Vec<(Binder<'tcx, TraitRef<'tcx>>, Span)>,
+        user_written_clauses: impl IntoIterator<Item = (ty::Clause<'tcx>, Span)>,
     ) -> ErrorGuaranteed {
         let tcx = self.tcx();
-        let trait_alias_span = trait_bounds
-            .iter()
-            .map(|&(trait_ref, _)| trait_ref.def_id())
-            .find(|&trait_ref| tcx.is_trait_alias(trait_ref))
-            .map(|trait_ref| tcx.def_span(trait_ref));
+        let trait_alias_span = user_written_clauses
+            .into_iter()
+            .filter_map(|(clause, _)| clause.as_trait_clause())
+            .find(|trait_ref| tcx.is_trait_alias(trait_ref.def_id()))
+            .map(|trait_ref| tcx.def_span(trait_ref.def_id()));
 
         self.dcx().emit_err(TraitObjectDeclaredWithNoTraits { span, trait_alias_span })
     }

compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs

@@ -1,6 +1,5 @@
 use std::cell::{Cell, RefCell};
 use std::cmp::max;
-use std::iter;
 use std::ops::Deref;
 
 use rustc_data_structures::fx::FxHashSet;
@@ -1009,11 +1008,11 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
 
         if self.tcx.is_trait_alias(trait_def_id) {
             // For trait aliases, recursively assume all explicitly named traits are relevant
-            for expansion in traits::expand_trait_aliases(
-                self.tcx,
-                iter::once((ty::Binder::dummy(trait_ref), self.span)),
-            ) {
-                let bound_trait_ref = expansion.trait_ref();
+            for (bound_trait_pred, _) in
+                traits::expand_trait_aliases(self.tcx, [(trait_ref.upcast(self.tcx), self.span)]).0
+            {
+                assert_eq!(bound_trait_pred.polarity(), ty::PredicatePolarity::Positive);
+                let bound_trait_ref = bound_trait_pred.map_bound(|pred| pred.trait_ref);
                 for item in self.impl_or_trait_item(bound_trait_ref.def_id()) {
                     if !self.has_applicable_self(&item) {
                         self.record_static_candidate(CandidateSource::Trait(

compiler/rustc_hir_typeck/src/method/probe.rs

@@ -437,7 +437,8 @@ pub fn report_dyn_incompatibility<'tcx>(
         tcx.dcx(),
         span,
         E0038,
-        "the trait `{}` cannot be made into an object",
+        "the {} `{}` cannot be made into an object",
+        tcx.def_descr(trait_def_id),
         trait_str
     );
     err.span_label(span, format!("`{trait_str}` cannot be made into an object"));

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

@@ -66,9 +66,9 @@ pub use self::specialize::{
 };
 pub use self::structural_normalize::StructurallyNormalizeExt;
 pub use self::util::{
-    BoundVarReplacer, PlaceholderReplacer, TraitAliasExpander, TraitAliasExpansionInfo, elaborate,
-    expand_trait_aliases, impl_item_is_final, supertraits,
-    transitive_bounds_that_define_assoc_item, upcast_choices, with_replaced_escaping_bound_vars,
+    BoundVarReplacer, PlaceholderReplacer, elaborate, expand_trait_aliases, impl_item_is_final,
+    supertraits, transitive_bounds_that_define_assoc_item, upcast_choices,
+    with_replaced_escaping_bound_vars,
 };
 use crate::error_reporting::InferCtxtErrorExt;
 use crate::infer::outlives::env::OutlivesEnvironment;

compiler/rustc_trait_selection/src/traits/mod.rs

@@ -1,162 +1,85 @@
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, VecDeque};
 
-use rustc_data_structures::fx::FxIndexMap;
-use rustc_errors::Diag;
+use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
 use rustc_hir::def_id::DefId;
 use rustc_infer::infer::InferCtxt;
 pub use rustc_infer::traits::util::*;
 use rustc_middle::bug;
 use rustc_middle::ty::{
-    self, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperFoldable, TypeVisitableExt, Upcast,
+    self, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperFoldable, TypeVisitableExt,
 };
 use rustc_span::Span;
 use smallvec::{SmallVec, smallvec};
 use tracing::debug;
 
-///////////////////////////////////////////////////////////////////////////
-// `TraitAliasExpander` iterator
-///////////////////////////////////////////////////////////////////////////
-
-/// "Trait alias expansion" is the process of expanding a sequence of trait
-/// references into another sequence by transitively following all trait
-/// aliases. e.g. If you have bounds like `Foo + Send`, a trait alias
-/// `trait Foo = Bar + Sync;`, and another trait alias
-/// `trait Bar = Read + Write`, then the bounds would expand to
-/// `Read + Write + Sync + Send`.
-/// Expansion is done via a DFS (depth-first search), and the `visited` field
-/// is used to avoid cycles.
-pub struct TraitAliasExpander<'tcx> {
-    tcx: TyCtxt<'tcx>,
-    stack: Vec<TraitAliasExpansionInfo<'tcx>>,
-}
-
-/// Stores information about the expansion of a trait via a path of zero or more trait aliases.
-#[derive(Debug, Clone)]
-pub struct TraitAliasExpansionInfo<'tcx> {
-    pub path: SmallVec<[(ty::PolyTraitRef<'tcx>, Span); 4]>,
-}
-
-impl<'tcx> TraitAliasExpansionInfo<'tcx> {
-    fn new(trait_ref: ty::PolyTraitRef<'tcx>, span: Span) -> Self {
-        Self { path: smallvec![(trait_ref, span)] }
-    }
-
-    /// Adds diagnostic labels to `diag` for the expansion path of a trait through all intermediate
-    /// trait aliases.
-    pub fn label_with_exp_info(
-        &self,
-        diag: &mut Diag<'_>,
-        top_label: &'static str,
-        use_desc: &str,
-    ) {
-        diag.span_label(self.top().1, top_label);
-        if self.path.len() > 1 {
-            for (_, sp) in self.path.iter().rev().skip(1).take(self.path.len() - 2) {
-                diag.span_label(*sp, format!("referenced here ({use_desc})"));
-            }
-        }
-        if self.top().1 != self.bottom().1 {
-            // When the trait object is in a return type these two spans match, we don't want
-            // redundant labels.
-            diag.span_label(
-                self.bottom().1,
-                format!("trait alias used in trait object type ({use_desc})"),
-            );
-        }
-    }
-
-    pub fn trait_ref(&self) -> ty::PolyTraitRef<'tcx> {
-        self.top().0
-    }
-
-    pub fn top(&self) -> &(ty::PolyTraitRef<'tcx>, Span) {
-        self.path.last().unwrap()
-    }
-
-    pub fn bottom(&self) -> &(ty::PolyTraitRef<'tcx>, Span) {
-        self.path.first().unwrap()
-    }
-
-    fn clone_and_push(&self, trait_ref: ty::PolyTraitRef<'tcx>, span: Span) -> Self {
-        let mut path = self.path.clone();
-        path.push((trait_ref, span));
-
-        Self { path }
-    }
-}
-
+/// Return the trait and projection predicates that come from eagerly expanding the
+/// trait aliases in the list of clauses. For each trait predicate, record a stack
+/// of spans that trace from the user-written trait alias bound. For projection predicates,
+/// just record the span of the projection itself.
+///
+/// For trait aliases, we don't deduplicte the predicates, since we currently do not
+/// consider duplicated traits as a single trait for the purposes of our "one trait principal"
+/// restriction; however, for projections we do deduplicate them.
+///
+/// ```rust,ignore (fails)
+/// trait Bar {}
+/// trait Foo = Bar + Bar;
+///
+/// let not_object_safe: dyn Foo; // bad, two `Bar` principals.
+/// ```
 pub fn expand_trait_aliases<'tcx>(
     tcx: TyCtxt<'tcx>,
-    trait_refs: impl Iterator<Item = (ty::PolyTraitRef<'tcx>, Span)>,
-) -> TraitAliasExpander<'tcx> {
-    let items: Vec<_> =
-        trait_refs.map(|(trait_ref, span)| TraitAliasExpansionInfo::new(trait_ref, span)).collect();
-    TraitAliasExpander { tcx, stack: items }
-}
-
-impl<'tcx> TraitAliasExpander<'tcx> {
-    /// If `item` is a trait alias and its predicate has not yet been visited, then expands `item`
-    /// to the definition, pushes the resulting expansion onto `self.stack`, and returns `false`.
-    /// Otherwise, immediately returns `true` if `item` is a regular trait, or `false` if it is a
-    /// trait alias.
-    /// The return value indicates whether `item` should be yielded to the user.
-    fn expand(&mut self, item: &TraitAliasExpansionInfo<'tcx>) -> bool {
-        let tcx = self.tcx;
-        let trait_ref = item.trait_ref();
-        let pred = trait_ref.upcast(tcx);
-
-        debug!("expand_trait_aliases: trait_ref={:?}", trait_ref);
-
-        // Don't recurse if this bound is not a trait alias.
-        let is_alias = tcx.is_trait_alias(trait_ref.def_id());
-        if !is_alias {
-            return true;
-        }
-
-        // Don't recurse if this trait alias is already on the stack for the DFS search.
-        let anon_pred = anonymize_predicate(tcx, pred);
-        if item
-            .path
-            .iter()
-            .rev()
-            .skip(1)
-            .any(|&(tr, _)| anonymize_predicate(tcx, tr.upcast(tcx)) == anon_pred)
-        {
-            return false;
-        }
-
-        // Get components of trait alias.
-        let predicates = tcx.explicit_super_predicates_of(trait_ref.def_id());
-        debug!(?predicates);
-
-        let items = predicates.skip_binder().iter().rev().filter_map(|(pred, span)| {
-            pred.instantiate_supertrait(tcx, trait_ref)
-                .as_trait_clause()
-                .map(|trait_ref| item.clone_and_push(trait_ref.map_bound(|t| t.trait_ref), *span))
-        });
-        debug!("expand_trait_aliases: items={:?}", items.clone().collect::<Vec<_>>());
-
-        self.stack.extend(items);
-
-        false
-    }
-}
-
-impl<'tcx> Iterator for TraitAliasExpander<'tcx> {
-    type Item = TraitAliasExpansionInfo<'tcx>;
-
-    fn size_hint(&self) -> (usize, Option<usize>) {
-        (self.stack.len(), None)
-    }
-
-    fn next(&mut self) -> Option<TraitAliasExpansionInfo<'tcx>> {
-        while let Some(item) = self.stack.pop() {
-            if self.expand(&item) {
-                return Some(item);
+    clauses: impl IntoIterator<Item = (ty::Clause<'tcx>, Span)>,
+) -> (
+    Vec<(ty::PolyTraitPredicate<'tcx>, SmallVec<[Span; 1]>)>,
+    Vec<(ty::PolyProjectionPredicate<'tcx>, Span)>,
+) {
+    let mut trait_preds = vec![];
+    let mut projection_preds = vec![];
+    let mut seen_projection_preds = FxHashSet::default();
+
+    let mut queue: VecDeque<_> = clauses.into_iter().map(|(p, s)| (p, smallvec![s])).collect();
+
+    while let Some((clause, spans)) = queue.pop_front() {
+        match clause.kind().skip_binder() {
+            ty::ClauseKind::Trait(trait_pred) => {
+                if tcx.is_trait_alias(trait_pred.def_id()) {
+                    queue.extend(
+                        tcx.explicit_super_predicates_of(trait_pred.def_id())
+                            .iter_identity_copied()
+                            .map(|(clause, span)| {
+                                let mut spans = spans.clone();
+                                spans.push(span);
+                                (
+                                    clause.instantiate_supertrait(
+                                        tcx,
+                                        clause.kind().rebind(trait_pred.trait_ref),
+                                    ),
+                                    spans,
+                                )
+                            }),
+                    );
+                } else {
+                    trait_preds.push((clause.kind().rebind(trait_pred), spans));
+                }
             }
+            ty::ClauseKind::Projection(projection_pred) => {
+                let projection_pred = clause.kind().rebind(projection_pred);
+                if !seen_projection_preds.insert(tcx.anonymize_bound_vars(projection_pred)) {
+                    continue;
+                }
+                projection_preds.push((projection_pred, *spans.last().unwrap()));
+            }
+            ty::ClauseKind::RegionOutlives(..)
+            | ty::ClauseKind::TypeOutlives(..)
+            | ty::ClauseKind::ConstArgHasType(_, _)
+            | ty::ClauseKind::WellFormed(_)
+            | ty::ClauseKind::ConstEvaluatable(_)
+            | ty::ClauseKind::HostEffect(..) => {}
         }
-        None
     }
+
+    (trait_preds, projection_preds)
 }
 
 ///////////////////////////////////////////////////////////////////////////