-
-
Notifications
You must be signed in to change notification settings - Fork 14.6k
Use trait_object_dummy_self more & heavily fix+update related docs
#153497
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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<'_>>> { | ||
|
|
@@ -153,7 +149,7 @@ fn insert_required_predicates_to_be_wf<'tcx>( | |
| args, | ||
| required_predicates, | ||
| explicit_map, | ||
| None, | ||
| IgnorePredicatesReferencingSelf::No, | ||
| ); | ||
| } | ||
|
|
||
|
|
@@ -174,31 +170,35 @@ fn insert_required_predicates_to_be_wf<'tcx>( | |
| alias.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; | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of a dummy |
||
| 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, | ||
| ); | ||
| } | ||
| } | ||
|
|
@@ -214,7 +214,7 @@ fn insert_required_predicates_to_be_wf<'tcx>( | |
| alias.args, | ||
| required_predicates, | ||
| explicit_map, | ||
| None, | ||
| IgnorePredicatesReferencingSelf::No, | ||
| ); | ||
| } | ||
|
|
||
|
|
@@ -243,76 +243,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<..>` | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've "moved" this lengthy explainer to the |
||
| // 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. | ||
|
Comment on lines
-288
to
-292
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The provided example ("we might then falsely conclude that e.g., Originally, it didn't say So the problem this paragraph is trying to convey is:
Therefore we filter out outlives-bounds mentioning However, using the trait object type itself is still problematic since it can lead to escaping bound vars (issue #53419), therefore it was replaced with the dummy |
||
| // | ||
| // 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()) | ||
|
Comment on lines
+262
to
+263
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that this whole "filter out things before substitution/instantiation" topic is completely irrelevant now due to us using So theoretically I could just look for fn param However, I feel like looking for |
||
| { | ||
| debug!("skipping self ty = {ty:?}"); | ||
| debug!("ignoring predicate since it references `Self`"); | ||
| continue; | ||
| } | ||
|
|
||
| let predicate = explicit_predicates.rebind(*outlives_predicate).instantiate(tcx, args); | ||
| 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); | ||
| 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 | ||
| /// | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -365,10 +365,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 | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Context: "converting" is old terminology for (HIR ty) lowering before the ASTconv → HIR ty lowering renaming. |
||
| /// 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`. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -330,14 +330,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> | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using a more Rust-like syntax instead of a Haskell-based one. |
||
| /// ``` | ||
| /// 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( | ||
|
|
@@ -380,13 +379,14 @@ 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()` | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
No longer up to date. First of all, Second of all, we no longer use |
||
| /// 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. | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're not mentioning |
||
| pub fn with_self_ty(self, interner: I, self_ty: I::Ty) -> TraitRef<I> { | ||
| // otherwise the escaping vars would be captured by the binder | ||
| // debug_assert!(!self_ty.has_escaping_bound_vars()); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This debug assert was accidentally commented out in a refactoring PR from 2018! See PR #53816, path |
||
| debug_assert!(!self_ty.has_escaping_bound_vars()); | ||
|
|
||
| TraitRef::new(interner, self.def_id, [self_ty.into()].into_iter().chain(self.args.iter())) | ||
| } | ||
|
|
@@ -397,10 +397,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)) | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(this parameter no longer exists, it's a local variable now)