Skip to content

Conversation

@cjgillot
Copy link
Contributor

Blocked on #103448
Specific diff https://github.com/cjgillot/rust/compare/no-late-fn..variance-rpit

This PR attempts to stop duplicating generic parameters between functions and opaque types.

fn foo<'a, 'b, T>() -> impl Sized + 'a {} // OLD fn foo<'a, 'b, T>() -> foo::<'static, 'static, T>::opaque::<'a> {} opaque foo::<T>::opaque<'a>: Sized + 'a; // OLD post #103448 fn foo<'a, 'b, T>() -> foo::<'static, 'static, T>::opaque::<'a> {} opaque foo::<'_a, '_b, T>::opaque<'a>: Sized + 'a; // NEW fn foo<'a, 'b, T>() -> foo::<'a, 'b, T>::opaque {} opaque foo::<'a, 'b, T>::opaque: Sized + 'a;

This allows to support Self in impl-trait, since we do not replace lifetimes by 'static any more.

The implementation relies on 2 tweaking rules for opaques in 2 places:

  • we only relate substs that correspond to captured lifetimes during TypeRelation;
  • we only list captured lifetimes in choice region computation.

For simplicity, I encoded the "capturedness" of lifetimes as a variance, Bivariant vs Invariant for unused vs captured lifetimes. The variances_of query used to ICE for opaques.

Writing the PR description made me realize that this strategy can be used without #103448 to support Self in RPIT. I'll probably make such a PR during the week.

r? types

@cjgillot cjgillot added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Oct 23, 2022
@rustbot rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Oct 23, 2022
@rustbot
Copy link
Collaborator

rustbot commented Oct 23, 2022

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in src/librustdoc/clean/types.rs

cc @camelid

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 23, 2022
@compiler-errors
Copy link
Member

compiler-errors commented Oct 23, 2022

This allows to support Self in impl-trait, since we do not replace lifetimes by 'static any more.

Can you elaborate on this? I don't have context of what impl Trait currently has issues with.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Oct 24, 2022

☔ The latest upstream changes (presumably #103337) made this pull request unmergeable. Please resolve the merge conflicts.

@rustbot
Copy link
Collaborator

rustbot commented Oct 26, 2022

The Miri subtree was changed

cc @rust-lang/miri

@rust-log-analyzer
Copy link
Collaborator

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

Click to see the possible cause of the failure (guessed by this bot)
configure: rust.debug-assertions := True configure: rust.overflow-checks := True configure: llvm.assertions := True configure: dist.missing-tools := True configure: build.configure-args := ['--enable-sccache', '--disable-manage-submodu ... configure: writing `config.toml` in current directory configure: configure: run `python /checkout/x.py --help` Attempting with retry: make prepare --- * highest error code: E0790 Found 506 error codes Found 0 error(s) in error codes Done! tidy error: /checkout/compiler/rustc_ast_lowering/src/lib.rs:1467: unexplained "```ignore" doctest; try one: * make the test actually pass, by adding necessary imports and declarations, or * use "```text", if the code is not Rust code, or * use "```compile_fail,Ennnn", if the code is expected to fail at compile time, or * use "```should_panic", if the code is expected to fail at run time, or * use "```no_run", if the code should type-check but not necessary linkable/runnable, or * explain it like "```ignore (cannot-test-this-because-xxxx)", if the annotation cannot be avoided. tidy error: /checkout/compiler/rustc_ast_lowering/src/lib.rs:1474: unexplained "```ignore" doctest; try one: * make the test actually pass, by adding necessary imports and declarations, or * use "```text", if the code is not Rust code, or * use "```compile_fail,Ennnn", if the code is expected to fail at compile time, or * use "```should_panic", if the code is expected to fail at run time, or * use "```no_run", if the code should type-check but not necessary linkable/runnable, or * explain it like "```ignore (cannot-test-this-because-xxxx)", if the annotation cannot be avoided. * 387 features some tidy checks failed Build completed unsuccessfully in 0:00:11 
@bors
Copy link
Collaborator

bors commented Oct 27, 2022

☔ The latest upstream changes (presumably #103623) made this pull request unmergeable. Please resolve the merge conflicts.

@compiler-errors compiler-errors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 7, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 21, 2022
Support using `Self` or projections inside an RPIT/async fn I reuse the same idea as rust-lang#103449 to use variances to encode whether a lifetime parameter is captured by impl-trait. The current implementation of async and RPIT replace all lifetimes from the parent generics by `'static`. This PR changes the scheme ```rust impl<'a> Foo<'a> { fn foo<'b, T>() -> impl Into<Self> + 'b { ... } } opaque Foo::<'_a>::foo::<'_b, T>::opaque<'b>: Into<Foo<'_a>> + 'b; impl<'a> Foo<'a> { // OLD fn foo<'b, T>() -> Foo::<'static>::foo::<'static, T>::opaque::<'b> { ... } ^^^^^^^ the `Self` becomes `Foo<'static>` // NEW fn foo<'b, T>() -> Foo::<'a>::foo::<'b, T>::opaque::<'b> { ... } ^^ the `Self` stays `Foo<'a>` } ``` There is the same issue with projections. In the example, substitute `Self` by `<T as Trait<'b>>::Assoc` in the sugared version, and `Foo<'_a>` by `<T as Trait<'_b>>::Assoc` in the desugared one. This allows to support `Self` in impl-trait, since we do not replace lifetimes by `'static` any more. The same trick allows to use projections like `T::Assoc` where `Self` is allowed. The feature is gated behind a `impl_trait_projections` feature gate. The implementation relies on 2 tweaking rules for opaques in 2 places: - we only relate substs that correspond to captured lifetimes during TypeRelation; - we only list captured lifetimes in choice region computation. For simplicity, I encoded the "capturedness" of lifetimes as a variance, `Bivariant` vs `Invariant` for unused vs captured lifetimes. The `variances_of` query used to ICE for opaques. Impl-trait that do not reference `Self` or projections will have their variances as: - `o` (invariant) for each parent type or const; - `*` (bivariant) for each parent lifetime --> will not participate in borrowck; - `o` (invariant) for each own lifetime. Impl-trait that does reference `Self` and/or projections will have some parent lifetimes marked as `o` (as the example above), and participate in type relation and borrowck. In the example above, `variances_of(opaque) = ['_a: o, '_b: *, T: o, 'b: o]`. r? types cc `@compiler-errors` , as you asked about the issue with `Self` and projections.
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Nov 27, 2022
Support using `Self` or projections inside an RPIT/async fn I reuse the same idea as rust-lang/rust#103449 to use variances to encode whether a lifetime parameter is captured by impl-trait. The current implementation of async and RPIT replace all lifetimes from the parent generics by `'static`. This PR changes the scheme ```rust impl<'a> Foo<'a> { fn foo<'b, T>() -> impl Into<Self> + 'b { ... } } opaque Foo::<'_a>::foo::<'_b, T>::opaque<'b>: Into<Foo<'_a>> + 'b; impl<'a> Foo<'a> { // OLD fn foo<'b, T>() -> Foo::<'static>::foo::<'static, T>::opaque::<'b> { ... } ^^^^^^^ the `Self` becomes `Foo<'static>` // NEW fn foo<'b, T>() -> Foo::<'a>::foo::<'b, T>::opaque::<'b> { ... } ^^ the `Self` stays `Foo<'a>` } ``` There is the same issue with projections. In the example, substitute `Self` by `<T as Trait<'b>>::Assoc` in the sugared version, and `Foo<'_a>` by `<T as Trait<'_b>>::Assoc` in the desugared one. This allows to support `Self` in impl-trait, since we do not replace lifetimes by `'static` any more. The same trick allows to use projections like `T::Assoc` where `Self` is allowed. The feature is gated behind a `impl_trait_projections` feature gate. The implementation relies on 2 tweaking rules for opaques in 2 places: - we only relate substs that correspond to captured lifetimes during TypeRelation; - we only list captured lifetimes in choice region computation. For simplicity, I encoded the "capturedness" of lifetimes as a variance, `Bivariant` vs `Invariant` for unused vs captured lifetimes. The `variances_of` query used to ICE for opaques. Impl-trait that do not reference `Self` or projections will have their variances as: - `o` (invariant) for each parent type or const; - `*` (bivariant) for each parent lifetime --> will not participate in borrowck; - `o` (invariant) for each own lifetime. Impl-trait that does reference `Self` and/or projections will have some parent lifetimes marked as `o` (as the example above), and participate in type relation and borrowck. In the example above, `variances_of(opaque) = ['_a: o, '_b: *, T: o, 'b: o]`. r? types cc `@compiler-errors` , as you asked about the issue with `Self` and projections.
@cjgillot cjgillot closed this Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-blocked Status: Blocked on something else such as an RFC or other implementation work. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

6 participants