Use trait_object_dummy_self more & heavily fix+update related docs#153497
Use trait_object_dummy_self more & heavily fix+update related docs#153497fmease wants to merge 1 commit intorust-lang:mainfrom
trait_object_dummy_self more & heavily fix+update related docs#153497Conversation
rustbot
added
S-waiting-on-author
|
|
||
| /// Infer predicates for the items in the crate. | ||
| /// | ||
| /// `global_inferred_outlives`: this is initially the empty map that |
There was a problem hiding this comment.
(this parameter no longer exists, it's a local variable now)
|
|
||
| /// 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()` |
There was a problem hiding this comment.
A common choice is
mk_err()
No longer up to date.
First of all, mk_err() no longer exists, it's Ty::new_error from middle which type IR shouldn't know about.
Second of all, we no longer use {type error} for that since that one carries an ErrorGuaranteed now, so it can no longer be used as a flexible dummy type. It's superseded by, you guessed it, trait_object_dummy_self (cc #70866, #71938).
| /// | ||
| /// 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. |
There was a problem hiding this comment.
We're not mentioning trait_object_dummy_self from middle by name because type IR shouldn't know about it.
| /// 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> |
There was a problem hiding this comment.
Using a more Rust-like syntax instead of a Haskell-based one.
| pub never: Ty<'tcx>, | ||
| pub self_param: Ty<'tcx>, | ||
|
|
||
| /// Dummy type used for the `Self` of a `TraitRef` created for converting |
There was a problem hiding this comment.
Context: "converting" is old terminology for (HIR ty) lowering before the ASTconv → HIR ty lowering renaming.
| /// 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> { | ||
| // otherwise the escaping vars would be captured by the binder | ||
| // debug_assert!(!self_ty.has_escaping_bound_vars()); |
There was a problem hiding this comment.
This debug assert was accidentally commented out in a refactoring PR from 2018! See PR #53816, path src/librustc/ty/sty.rs.
| 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<..>` |
There was a problem hiding this comment.
I've "moved" this lengthy explainer to the ty::Dynamic branch (or rather replaced it with a more concise & up to date one) because it is about trait object types specifically and therefore shouldn't belong in this general function!
| // 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. |
There was a problem hiding this comment.
The provided example ("we might then falsely conclude that e.g., X: 'x was a reasonable inferred requirement") doesn't make any sense whatsoever! That's because this comment wasn't properly updated over time:
Originally, it didn't say Self = usize but Self = dyn Trait<'x, X> which makes a whole lot more sense since back in ancient times, we did use the trait object type itself for the erased self type which was problematic and thus changed to usize (sic!).
So the problem this paragraph is trying to convey is:
- Given:
struct Struct<'x, X>(Box<dyn Trait<'x, X>>);trait Trait<'a, T> where Self: 'a {}
- We'd convert existential trait ref
Trait<'x, X>to normal trait ref<dyn Trait<'x, X> as Trait<'x, X>>(setting the self type to the trait object type itself) - Extract its args, so
[dyn Trait<'x, X>, 'x, X] - Instantiate
Self: 'afrom the trait with these args to obtaindyn Trait<'x, X>: 'x - We then split the trait object type into outlives-components, namely
X: 'x(and'x: 'x) which would be bad.
Therefore we filter out outlives-bounds mentioning Self before substitution/instantiation.
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 usize (PR #53441) but the comment wasn't updated (PR #56003 later updated it to Self = usize but didn't update the rest of the paragraph).
| // 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; |
There was a problem hiding this comment.
Instead of a dummy usize we now use the "canonical" trait_object_dummy_self which is way less janky.
| if let IgnorePredicatesReferencingSelf::Yes = ignore_preds_refing_self | ||
| && arg.walk().any(|arg| arg == tcx.types.self_param.into()) |
There was a problem hiding this comment.
Note that this whole "filter out things before substitution/instantiation" topic is completely irrelevant now due to us using trait_object_dummy_self instead of usize. The former is guaranteed to never occur in the outlives-predicates (that's a guarantee by HIR ty lowering / HIR analysis's type collection). The latter, usize, can obviously occur in the predicates.
So theoretically I could just look for fn param ignored_self_ty (set to the dummy Self for branch ty::Dynamic) after instantiation if I so wanted to.
However, I feel like looking for self_param before instantiation plus just having a Boolean-like parameter is a lot cleaner.
rustbot
added
the
S-waiting-on-review
rustbot
removed
the
S-waiting-on-author
|
rustbot has assigned @dingxiangfei2009. Use Why was this reviewer chosen?The reviewer was selected based on:
|
fmease
force-pushed
the
trait-obj-dummy-self-improvs
branch
3 times, most recently
from
03b5e8e to
4e7b116
Compare
This comment has been minimized.
This comment has been minimized.
fmease
force-pushed
the
trait-obj-dummy-self-improvs
branch
from
4e7b116 to
8507598
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
Are you telling me that this assertion is now triggering after a rebase whereas a month ago CI passed despite having been commented out for 8 years (#153497 (comment))? Are you kidding me? :( @ reviewer, feel free to ignore the failing assertion when reviewing my code (it's not relevant for the rest of this PR), I'll investigate it some other time because I don't have the energy right now. I also don't mind commenting it out again, adding a FIXME and opening an issue. |
|
☔ The latest upstream changes (presumably #154727) made this pull request unmergeable. Please resolve the merge conflicts. |
4 participants
trait_object_dummy_self(Infer(FreshTy(0))) in more placesTy::new_fresh(tcx, 0))usizeas a dummy self type!trait_object_dummy_self