Use trait_object_dummy_self more & heavily fix+update related docs

Author: fmease

compiler/rustc_hir_analysis/src/outlives/implicit_infer.rs

@@ -8,11 +8,7 @@ use tracing::debug;
 use super::explicit::ExplicitPredicatesMap;
 use super::utils::*;
 
-/// Infer predicates for the items in the crate.
-///
-/// `global_inferred_outlives`: this is initially the empty map that
-///     was generated by walking the items in the crate. This will
-///     now be filled with inferred predicates.
+/// Infer outlives-predicates for the items in the local crate.
 pub(super) fn infer_predicates(
     tcx: TyCtxt<'_>,
 ) -> FxIndexMap<DefId, ty::EarlyBinder<'_, RequiredPredicates<'_>>> {
@@ -154,7 +150,7 @@ fn insert_required_predicates_to_be_wf<'tcx>(
                     args,
                     required_predicates,
                     explicit_map,
-                    None,
+                    IgnorePredicatesReferencingSelf::No,
                 );
             }
 
@@ -175,31 +171,35 @@ fn insert_required_predicates_to_be_wf<'tcx>(
                     args,
                     required_predicates,
                     explicit_map,
-                    None,
+                    IgnorePredicatesReferencingSelf::No,
                 );
             }
 
             ty::Dynamic(obj, ..) => {
                 // This corresponds to `dyn Trait<..>`. In this case, we should
                 // use the explicit predicates as well.
                 debug!("Dynamic");
-                if let Some(ex_trait_ref) = obj.principal() {
-                    // Here, we are passing the type `usize` as a
-                    // placeholder value with the function
-                    // `with_self_ty`, since there is no concrete type
-                    // `Self` for a `dyn Trait` at this
-                    // stage. Therefore when checking explicit
-                    // predicates in `check_explicit_predicates` we
-                    // need to ignore checking the explicit_map for
-                    // Self type.
-                    let args = ex_trait_ref.with_self_ty(tcx, tcx.types.usize).skip_binder().args;
+                if let Some(trait_ref) = obj.principal() {
+                    let args = trait_ref
+                        .with_self_ty(tcx, tcx.types.trait_object_dummy_self)
+                        .skip_binder()
+                        .args;
+                    // We skip predicates that reference the `Self` type parameter since we don't
+                    // want to leak the dummy Self to the predicates map.
+                    //
+                    // While filtering out bounds like `Self: 'a` as in `trait Trait<'a, T>: 'a {}`
+                    // doesn't matter since they can't affect the lifetime / type parameters anyway,
+                    // for bounds like `Self::AssocTy: 'b` which we of course currently also ignore
+                    // (see also #54467) it might conceivably be better to extract the binding
+                    // `AssocTy = U` from the trait object type (which must exist) and thus infer
+                    // an outlives requirement that `U: 'b`.
                     check_explicit_predicates(
                         tcx,
-                        ex_trait_ref.skip_binder().def_id,
+                        trait_ref.def_id(),
                         args,
                         required_predicates,
                         explicit_map,
-                        Some(tcx.types.self_param),
+                        IgnorePredicatesReferencingSelf::Yes,
                     );
                 }
             }
@@ -215,7 +215,7 @@ fn insert_required_predicates_to_be_wf<'tcx>(
                     args,
                     required_predicates,
                     explicit_map,
-                    None,
+                    IgnorePredicatesReferencingSelf::No,
                 );
             }
 
@@ -244,77 +244,44 @@ fn insert_required_predicates_to_be_wf<'tcx>(
 /// will give us `U: 'static` and `U: Outer`. The latter we
 /// can ignore, but we will want to process `U: 'static`,
 /// applying the instantiation as above.
+#[tracing::instrument(level = "debug", skip(tcx))]
 fn check_explicit_predicates<'tcx>(
     tcx: TyCtxt<'tcx>,
     def_id: DefId,
     args: &[GenericArg<'tcx>],
     required_predicates: &mut RequiredPredicates<'tcx>,
     explicit_map: &mut ExplicitPredicatesMap<'tcx>,
-    ignored_self_ty: Option<Ty<'tcx>>,
+    ignore_preds_refing_self: IgnorePredicatesReferencingSelf,
 ) {
-    debug!(
-        "check_explicit_predicates(def_id={:?}, \
-         args={:?}, \
-         explicit_map={:?}, \
-         required_predicates={:?}, \
-         ignored_self_ty={:?})",
-        def_id, args, explicit_map, required_predicates, ignored_self_ty,
-    );
     let explicit_predicates = explicit_map.explicit_predicates_of(tcx, def_id);
 
-    for (outlives_predicate, &span) in explicit_predicates.as_ref().skip_binder() {
-        debug!("outlives_predicate = {outlives_predicate:?}");
-
-        // Careful: If we are inferring the effects of a `dyn Trait<..>`
-        // type, then when we look up the predicates for `Trait`,
-        // we may find some that reference `Self`. e.g., perhaps the
-        // definition of `Trait` was:
-        //
-        // ```
-        // trait Trait<'a, T> where Self: 'a  { .. }
-        // ```
-        //
-        // we want to ignore such predicates here, because
-        // there is no type parameter for them to affect. Consider
-        // a struct containing `dyn Trait`:
-        //
-        // ```
-        // struct MyStruct<'x, X> { field: Box<dyn Trait<'x, X>> }
-        // ```
-        //
-        // The `where Self: 'a` predicate refers to the *existential, hidden type*
-        // that is represented by the `dyn Trait`, not to the `X` type parameter
-        // (or any other generic parameter) declared on `MyStruct`.
-        //
-        // Note that we do this check for self **before** applying `args`. In the
-        // case that `args` come from a `dyn Trait` type, our caller will have
-        // included `Self = usize` as the value for `Self`. If we were
-        // to apply the args, and not filter this predicate, we might then falsely
-        // conclude that e.g., `X: 'x` was a reasonable inferred requirement.
-        //
-        // Another similar case is where we have an inferred
-        // requirement like `<Self as Trait>::Foo: 'b`. We presently
-        // ignore such requirements as well (cc #54467)-- though
-        // conceivably it might be better if we could extract the `Foo
-        // = X` binding from the object type (there must be such a
-        // binding) and thus infer an outlives requirement that `X:
-        // 'b`.
-        if let Some(self_ty) = ignored_self_ty
-            && let GenericArgKind::Type(ty) = outlives_predicate.0.kind()
-            && ty.walk().any(|arg| arg == self_ty.into())
+    for (&predicate @ ty::OutlivesPredicate(arg, _), &span) in
+        explicit_predicates.as_ref().skip_binder()
+    {
+        debug!(?predicate);
+
+        if let IgnorePredicatesReferencingSelf::Yes = ignore_preds_refing_self
+            && arg.walk().any(|arg| arg == tcx.types.self_param.into())
         {
-            debug!("skipping self ty = {ty:?}");
+            debug!("ignoring predicate since it references `Self`");
             continue;
         }
 
-        let predicate =
-            explicit_predicates.rebind(*outlives_predicate).instantiate(tcx, args).skip_norm_wip();
-        debug!("predicate = {predicate:?}");
-        insert_outlives_predicate(tcx, predicate.0, predicate.1, span, required_predicates);
+        let predicate @ ty::OutlivesPredicate(arg, region) =
+            explicit_predicates.rebind(predicate).instantiate(tcx, args).skip_norm_wip();
+        debug!(?predicate);
+
+        insert_outlives_predicate(tcx, arg, region, span, required_predicates);
     }
 }
 
-/// Check the inferred predicates declared on the type.
+#[derive(Debug)]
+enum IgnorePredicatesReferencingSelf {
+    Yes,
+    No,
+}
+
+/// Check the inferred predicates of the type.
 ///
 /// ### Example
 ///

compiler/rustc_middle/src/ty/context.rs

@@ -324,10 +324,32 @@ pub struct CommonTypes<'tcx> {
     pub never: Ty<'tcx>,
     pub self_param: Ty<'tcx>,
 
-    /// Dummy type used for the `Self` of a `TraitRef` created for converting
-    /// a trait object, and which gets removed in `ExistentialTraitRef`.
-    /// This type must not appear anywhere in other converted types.
-    /// `Infer(ty::FreshTy(0))` does the job.
+    /// A dummy type that can be used as the self type of trait object types outside of
+    /// [`ty::ExistentialTraitRef`], [`ty::ExistentialProjection`], etc.
+    ///
+    /// This is most useful or even necessary when you want to manipulate existential predicates
+    /// together with normal predicates or if you want to pass them to an API that only expects
+    /// normal predicates.
+    ///
+    /// Indeed, you can sometimes use the trait object type itself as the self type instead of this
+    /// dummy type. However, that's not always correct: For example, if said trait object type can
+    /// also appear "naturally" in whatever type system entity you're working with (like predicates)
+    /// but you still need to be able to identify the erased self type later on.
+    /// That's when this dummy type comes in handy.
+    ///
+    /// HIR ty lowering guarantees / has to guarantee that this dummy type doesn't appear in the
+    /// lowered types, so you can "freely" use it (see warning below).
+    ///
+    /// <div class="warning">
+    ///
+    /// Under the hood, this type is just `ty::Infer(ty::FreshTy(0))`. Consequently, you must be
+    /// sure that fresh types cannot appear by other means in whatever type system entity you're
+    /// working with.
+    ///
+    /// Keep uses of this dummy type as local as possible and try not to leak it to subsequent
+    /// passes!
+    ///
+    /// </div>
     pub trait_object_dummy_self: Ty<'tcx>,
 
     /// Pre-interned `Infer(ty::TyVar(n))` for small values of `n`.

compiler/rustc_middle/src/ty/print/pretty.rs

@@ -3265,9 +3265,9 @@ define_print! {
     }
 
     ty::ExistentialTraitRef<'tcx> {
-        // Use a type that can't appear in defaults of type parameters.
-        let dummy_self = Ty::new_fresh(p.tcx(), 0);
-        let trait_ref = self.with_self_ty(p.tcx(), dummy_self);
+        // Dummy Self is safe to use as it can't appear in generic param defaults which is important
+        // later on for correctly eliding generic args that coincide with their default.
+        let trait_ref = self.with_self_ty(p.tcx(), p.tcx().types.trait_object_dummy_self);
         trait_ref.print_only_trait_path().print(p)?;
     }
 

compiler/rustc_symbol_mangling/src/v0.rs

@@ -646,9 +646,11 @@ impl<'tcx> Printer<'tcx> for V0SymbolMangler<'tcx> {
                 // could have different bound vars *anyways*.
                 match predicate.as_ref().skip_binder() {
                     ty::ExistentialPredicate::Trait(trait_ref) => {
-                        // Use a type that can't appear in defaults of type parameters.
-                        let dummy_self = Ty::new_fresh(p.tcx, 0);
-                        let trait_ref = trait_ref.with_self_ty(p.tcx, dummy_self);
+                        // Dummy Self is safe to use as it can't appear in generic param defaults
+                        // which is important later on for correctly eliding generic args that
+                        // coincide with their default.
+                        let trait_ref =
+                            trait_ref.with_self_ty(p.tcx, p.tcx.types.trait_object_dummy_self);
                         p.print_def_path(trait_ref.def_id, trait_ref.args)?;
                     }
                     ty::ExistentialPredicate::Projection(projection) => {

compiler/rustc_type_ir/src/predicate.rs

@@ -388,14 +388,13 @@ impl<I: Interner> ty::Binder<I, ExistentialPredicate<I>> {
     }
 }
 
-/// An existential reference to a trait, where `Self` is erased.
+/// An existential reference to a trait where the self type `Self` is erased.
 ///
-/// For example, the trait object `Trait<'a, 'b, X, Y>` is:
+/// For example, the trait object type `Trait<'a, T, N>` can be understood as:
 /// ```ignore (illustrative)
-/// exists T. T: Trait<'a, 'b, X, Y>
+/// exists<X> X: Trait<'a, T, N>
 /// ```
-/// The generic parameters don't include the erased `Self`, only trait
-/// type and lifetime parameters (`[X, Y]` and `['a, 'b]` above).
+/// The generic arguments don't include the erased self type (so it's only `['a, T, N]`).
 #[derive_where(Clone, Copy, Hash, PartialEq; I: Interner)]
 #[derive(TypeVisitable_Generic, GenericTypeVisitable, TypeFoldable_Generic, Lift_Generic)]
 #[cfg_attr(
@@ -438,13 +437,18 @@ impl<I: Interner> ExistentialTraitRef<I> {
         }
     }
 
-    /// Object types don't have a self type specified. Therefore, when
-    /// we convert the principal trait-ref into a normal trait-ref,
-    /// you must give *some* self type. A common choice is `mk_err()`
-    /// or some placeholder type.
+    /// Convert the *existential* trait ref into a normal one by providing a self type.
+    ///
+    /// Existential trait refs don't contain a self type, it's erased.
+    /// Therefore, you must specify *some* self type to perform the conversion.
+    /// A common choice is the trait object type itself or some kind of dummy type.
     pub fn with_self_ty(self, interner: I, self_ty: I::Ty) -> TraitRef<I> {
+        // FIXME(#157122): This assertion was accidentally commented out in refactoring PR #53816
+        //                 back in 2018 but nowadays it can actually trigger. Either remove this
+        //                 comment entirely if the assertion is incorrect or uncomment it and fix
+        //                 the fallout!
         // otherwise the escaping vars would be captured by the binder
-        // debug_assert!(!self_ty.has_escaping_bound_vars());
+        //debug_assert!(!self_ty.has_escaping_bound_vars());
 
         TraitRef::new(interner, self.def_id, [self_ty.into()].into_iter().chain(self.args.iter()))
     }
@@ -455,10 +459,9 @@ impl<I: Interner> ty::Binder<I, ExistentialTraitRef<I>> {
         self.skip_binder().def_id
     }
 
-    /// Object types don't have a self type specified. Therefore, when
-    /// we convert the principal trait-ref into a normal trait-ref,
-    /// you must give *some* self type. A common choice is `mk_err()`
-    /// or some placeholder type.
+    /// Convert the *existential* polymorphic trait ref into a normal one by providing a self type.
+    ///
+    /// See also [`ExistentialTraitRef::with_self_ty`].
     pub fn with_self_ty(&self, cx: I, self_ty: I::Ty) -> ty::Binder<I, TraitRef<I>> {
         self.map_bound(|trait_ref| trait_ref.with_self_ty(cx, self_ty))
     }