Experiment: Reborrow traits

[aapoalas]

View all comments

With this PR we now have basic functional Reborrow and CoerceShared traits. The current limitations are:

1. Reborrowable types can only have one lifetime parameter, so as to avoid having to figure out and store in rmeta the information of which lifetimes weaken during reborrowing.
2. Reborrowing of &mut wrappers is working (though I've not tested generic wrappers like Option<&mut T> yet), but CoerceShared of &mut wrappers currently causes an ICE.

The remaining tasks to complete before I'd consider this PR mergeable are:

• Fix ICE on CoerceShared. Unfortunately this might require dipping into rmeta.
• Expand the tests to give a more complete view of the current state of the experiment.

Reborrow traits experiment: #145612

Co-authored by @dingxiangfei2009

[rustbot]

Some changes occurred in rustc_ty_utils::consts.rs

cc @BoxyUwU

Some changes occurred in match checking

cc @Nadrieril

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @vakaras

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a, @makai410

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred to constck

cc @fee1-dead

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

[aapoalas]

r? compiler

[BoxyUwU]

Where can I read context on what CoerceShared is/why it exists, and what the design for reborrow traits is

[aapoalas]

Where can I read context on what CoerceShared is/why it exists, and what the design for reborrow traits is

I added a link to the experiment tracking issue but put it very shortly: Reborrow is to struct X<'a>(..) what &mut *x is for &mut T, and CoerceShared is the equivalent operation as &*x. CoerceShared takes a type argument instead of having an associated type in expectation of it being possible to reborrow types like struct Double(&mut A, &mut B) which would then have three different "shared reference" variants: struct DR1(&A, &mut B), struct DR2(&mut A, &B), and struct DR3(&A, &B), though I am currently doubting the wisdom of this choice and it may return to using an associated type.

[BoxyUwU]

I think I'll just
r? me
on this since I know jack is quite busy with coercions work right now and this should probably have a types reviewer 🤔

[aapoalas]

I think I'll just r? me on this since I know jack is quite busy with coercions work right now and this should probably have a types reviewer 🤔

Thank you <3 my top secret plan (designed together with tmandry) was to get a compiler review and then reroll into types to that view as well so that works well :)

[jackh726]

I think I'll just r? me on this since I know jack is quite busy with coercions work right now and this should probably have a types reviewer 🤔

Appreciated.

[oli-obk]

Not sure this would be better, but if there are also plans to generalize field projections later, it would probably be good to try to keep references and custom ref types in sync

View changes since this review

[aapoalas]

@rustbot ready

[RalfJung]

I don't have anything more to add here, most of this touches code I know nothing about. I deliberately unsubscribed from the PR.

[wesleywiser]

Hi @oli-obk, it looks like this is ready for another round of review when you get a chance. Thanks! 🙂

[rustbot]

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.

[oli-obk]

I don't understand this comment. And why is it on the destructuring statement?

View changes since the review

[aapoalas]

This is directly copied over from add_reborrow_constraint where the same comment is in a similarly odd place.

I can remove it if preferred; I thought I'd just keep it same since I don't understand it myself either :D

[oli-obk]

wait, but this is validation :D

double-checking seems good, especially to catch optimizations from messing up

View changes since the review

[aapoalas]

Yeah; the thinking here is that because we only produce an Rvalue::Reborrow from an adjustment, and we only produce such an adjustment if we're looking at an assignment where we're going either from T -> T where T: Reborrow, or we're going from T -> U where T: CoerceShared<U>, then at this point we know we're necessarily doing a T -> T or T -> U and types match.

But double-checking is fine by me, and it does also offer up a future improvement of the implementation where we drop the adjustment and instead perform Reborrow somewhere at a deeper level on every use/access of a Reborrow or CoerceShared type.

[oli-obk]

how much is the logic of this copied from similar logic on the other coercion traits? Can we share some of the algorithm or sth else here?

View changes since the review

[aapoalas]

The basic structure and error handling is copied from CoerceUnsized. I'll take a look at possible deduplications.

[rust-log-analyzer]

The job pr-check-2 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

    Checking rustc_codegen_llvm v0.0.0 (/checkout/compiler/rustc_codegen_llvm)
error[E0308]: mismatched types
   --> compiler/rustc_hir_analysis/src/coherence/builtin.rs:473:9
    |
470 |     let Ok(normalized_ty) = ocx.structurally_normalize_ty(
    |                                 ------------------------- arguments to this method are incorrect
...
473 |         ty,
    |         ^^ expected `Unnormalized<TyCtxt<'_>, Ty<'_>>`, found `Ty<'_>`
    |
    = note: expected struct `Unnormalized<rustc_middle::ty::TyCtxt<'_>, rustc_middle::ty::Ty<'_>>`
               found struct `rustc_middle::ty::Ty<'tcx>`
note: method defined here
   --> compiler/rustc_trait_selection/src/traits/engine.rs:354:12
    |
354 |     pub fn structurally_normalize_ty(
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0609]: no field `def_id` on type `Unnormalized<TyCtxt<'_>, TraitRef<TyCtxt<'_>>>`
   --> compiler/rustc_hir_analysis/src/coherence/builtin.rs:519:26
    |
519 |     assert_eq!(trait_ref.def_id, reborrow_trait);
    |                          ^^^^^^ unknown field
    |
    = note: the full name for the type has been written to '/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu/release/deps/rustc_hir_analysis-c9805121477c5937.long-type-17232082633410946243.txt'
    = note: consider using `--verbose` to print the full type name to the console

error[E0599]: the method `has_escaping_bound_vars` exists for struct `Unnormalized<rustc_middle::ty::TyCtxt<'_>, rustc_middle::ty::Ty<'_>>`, but its trait bounds were not satisfied
   --> compiler/rustc_hir_analysis/src/coherence/builtin.rs:521:21
    |
521 |     assert!(!source.has_escaping_bound_vars());
    |                     ^^^^^^^^^^^^^^^^^^^^^^^ method cannot be called due to unsatisfied trait bounds
    |
   ::: compiler/rustc_type_ir/src/unnormalized.rs:28:1
    |
 28 | pub struct Unnormalized<I: Interner, T> {
    | --------------------------------------- doesn't satisfy `_: TypeVisitable<_>` or `_: TypeVisitableExt<_>`
    |
    = note: the following trait bounds were not satisfied:
            `Unnormalized<rustc_middle::ty::TyCtxt<'_>, rustc_middle::ty::Ty<'_>>: TypeVisitable<_>`
            which is required by `Unnormalized<rustc_middle::ty::TyCtxt<'_>, rustc_middle::ty::Ty<'_>>: TypeVisitableExt<_>`

error[E0599]: the method `kind` exists for struct `Unnormalized<rustc_middle::ty::TyCtxt<'_>, rustc_middle::ty::Ty<'_>>`, but its trait bounds were not satisfied
   --> compiler/rustc_hir_analysis/src/coherence/builtin.rs:523:36
    |
523 |     let (def, args) = match source.kind() {
    |                                    ^^^^ method cannot be called due to unsatisfied trait bounds
    |
   ::: compiler/rustc_middle/src/ty/context.rs:803:1
    |
803 | pub struct TyCtxt<'tcx> {
    | ----------------------- doesn't satisfy `<_ as Interner>::Clause = Ty<'_>`
    |
    = note: the following trait bounds were not satisfied:
            `<rustc_middle::ty::TyCtxt<'_> as Interner>::Clause = rustc_middle::ty::Ty<'_>`

error[E0609]: no field `def_id` on type `Unnormalized<TyCtxt<'_>, TraitRef<TyCtxt<'_>>>`
   --> compiler/rustc_hir_analysis/src/coherence/builtin.rs:634:26
    |
634 |     assert_eq!(trait_ref.def_id, coerce_shared_trait);
    |                          ^^^^^^ unknown field
    |
    = note: the full name for the type has been written to '/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu/release/deps/rustc_hir_analysis-c9805121477c5937.long-type-17232082633410946243.txt'
    = note: consider using `--verbose` to print the full type name to the console

error[E0609]: no field `args` on type `Unnormalized<TyCtxt<'_>, TraitRef<TyCtxt<'_>>>`
   --> compiler/rustc_hir_analysis/src/coherence/builtin.rs:636:74
    |
636 |         structurally_normalize_ty(tcx, &infcx, impl_did, span, trait_ref.args.type_at(1))
    |                                                                          ^^^^ unknown field
    |
    = note: the full name for the type has been written to '/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu/release/deps/rustc_hir_analysis-c9805121477c5937.long-type-17232082633410946243.txt'
    = note: consider using `--verbose` to print the full type name to the console

error[E0599]: the method `has_escaping_bound_vars` exists for struct `Unnormalized<rustc_middle::ty::TyCtxt<'_>, rustc_middle::ty::Ty<'_>>`, but its trait bounds were not satisfied
   --> compiler/rustc_hir_analysis/src/coherence/builtin.rs:642:21
    |
642 |     assert!(!source.has_escaping_bound_vars());
    |                     ^^^^^^^^^^^^^^^^^^^^^^^ method cannot be called due to unsatisfied trait bounds
    |
   ::: compiler/rustc_type_ir/src/unnormalized.rs:28:1
    |
 28 | pub struct Unnormalized<I: Interner, T> {
    | --------------------------------------- doesn't satisfy `_: TypeVisitable<_>` or `_: TypeVisitableExt<_>`
    |
    = note: the following trait bounds were not satisfied:
            `Unnormalized<rustc_middle::ty::TyCtxt<'_>, rustc_middle::ty::Ty<'_>>: TypeVisitable<_>`
            which is required by `Unnormalized<rustc_middle::ty::TyCtxt<'_>, rustc_middle::ty::Ty<'_>>: TypeVisitableExt<_>`

error[E0599]: the method `kind` exists for struct `Unnormalized<rustc_middle::ty::TyCtxt<'_>, rustc_middle::ty::Ty<'_>>`, but its trait bounds were not satisfied
   --> compiler/rustc_hir_analysis/src/coherence/builtin.rs:644:30
    |
644 |     let data = match (source.kind(), target.kind()) {
    |                              ^^^^ method cannot be called due to unsatisfied trait bounds
    |
   ::: compiler/rustc_middle/src/ty/context.rs:803:1
    |
803 | pub struct TyCtxt<'tcx> {
    | ----------------------- doesn't satisfy `<_ as Interner>::Clause = Ty<'_>`
    |
    = note: the following trait bounds were not satisfied:
            `<rustc_middle::ty::TyCtxt<'_> as Interner>::Clause = rustc_middle::ty::Ty<'_>`

Some errors have detailed explanations: E0308, E0599, E0609.
For more information about an error, try `rustc --explain E0308`.
[RUSTC-TIMING] rustc_hir_analysis test:false 6.167
error: could not compile `rustc_hir_analysis` (lib) due to 8 previous errors