Skip to content
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

Stabilize associated type position impl Trait (ATPIT) #120700

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

traviscross
Copy link
Contributor

@traviscross traviscross commented Feb 6, 2024

Stabilization report

Summary

We are stabilizing #![feature(impl_trait_in_assoc_type)], commonly called either "associated type position impl Trait" (ATPIT) or "impl Trait in associated type" (ITIAT).

Among other things, this feature allows async blocks and async fn to be used in more places (instead of writing implementations of Future by hand), filling gaps in the story of async Rust.

Stabilizing ATPIT helps the many Rust users who have well known use cases, including, e.g., the Tower library (in particular, e.g., its Service trait) and its extensive ecosystem.

The theme of this stabilization report is simplicity. Through much work, we've found that we can stabilize a subset of RFC 2515 that solves the most demanded use cases while enabling a simple and robust implementation in the compiler, supporting efficient implementations in other tooling, and answering all previously-open language design questions in a principled way.

This is a partial stabilization of RFC 2515 and #63063.

What is stabilized

Summary of stabilization

We now allow impl Trait to appear in type position in associated type definitions within trait implementations.

For the first time, we can now use async blocks to implement IntoFuture without boxing. E.g.:

use core::future::{Future, IntoFuture};

struct AlwaysAsync42;
impl IntoFuture for AlwaysAsync42 {
    type Output = impl Iterator<Item = impl Fn() -> u8>;
    type IntoFuture = impl Future<Output = Self::Output>;
    fn into_future(self) -> Self::IntoFuture { async {
        core::iter::repeat(|| 42)
    }}
}

Just as with RPIT, impl Trait may appear syntactically multiple times within the type, and any item that does not register a hidden type for the opaque witnesses it as an opaque type. No items outside of the impl block may register hidden types for impl Trait opaques defined within. The rules for which items within the impl block may do so are a simple extension of the RPIT rules and are described below.

Opaqueness

When we say that an item can only use a type opaquely, what we mean is that item can only use the type via the interfaces defined by the traits that the type is declared to implement and those of the leaked auto traits (as with RPIT).

A type that can only be used opaquely by some items is known as an opaque type, or, in context, as simply an opaque. The concrete type or type constructor that underlies the opaque is known as the hidden type, or, in context, as the hidden.

When an item chooses a concrete hidden type to underlie some opaque, we say that it has registered a hidden for that opaque, or equivalently, that it has "defined the hidden type of the opaque", or, more loosely and in context, has "defined the opaque type".

We describe below which items may and must register a hidden type for a given opaque. Other items may only use such types opaquely.

Parallel to RPIT

ATPIT is the extension of RPIT to associated type definitions and trait implementations. Everything that is true about RPIT opaque types is true about ATPIT opaque types modulo that:

  • For RPIT, when an impl Trait opaque type appears in the signature of an item, that item may and must register a hidden type for that opaque.
  • For ATPIT, when an impl Trait opaque type in the same impl block is syntactically reachable from an associated type in the signature of an item, that item may and must register a hidden type for that opaque.

Syntactic reachability rule

The rule to decide whether an item may and must register a hidden type for an opaque is strictly syntactic and strictly local to the impl block. It's an extension from considering just the signature, as with RPIT, to also considering the definitions of associated types in the same impl.

Intuitively, we collect all impl Trait types within the same trait impl that are syntactically reachable from the signature of an item, considering only that item's signature and the associated type definitions within the impl block. We don't recurse into ADTs (e.g. the fields of a struct).

More precisely, the rule is as follows:

To determine the set of impl Trait opaque types for which an item may and must register a hidden type, from the signature (including from any where clauses), we:

  1. Collect all types and generic type arguments1 without normalization, and for each, we:

    • a. Collect RPIT-like ("existential") uses of impl Trait into the set of opaque types for which this item may and must register a hidden type.2

    • b. Recurse syntactically (i.e. normalize one step) into the definition of associated types from the same trait when all of the generic arguments (including Self) match, and from there, we repeat step 1.

Note that RPIT already performs steps 1 and 1a. The stabilization of ATPIT adds only step 1b.

Example of syntactic reachability rule

Following the rules for syntactic reachability, this works as we would expect:

use core::future::{Future, IntoFuture};

struct AlwaysAsync42;
impl IntoFuture for AlwaysAsync42 {
    type Output = Box<dyn Iterator<Item = impl Fn() -> u8>>;
    type IntoFuture =
        impl Future<Output = <AlwaysAsync42 as IntoFuture>::Output>;
    fn into_future(self) -> Self::IntoFuture { async {
        Box::new(core::iter::repeat(|| 42u8))
            as Box<dyn Iterator<Item = _>>
    }}
}

May define == must define rule

If an impl Trait opaque type is syntactically reachable from the signature of an item according to the syntactic reachability rule, then a hidden type may and must be registered by that item for the opaque.

Items not satisfying this predicate may not register a hidden type for the opaque.

Sibling only rule

Only the syntactic items that are direct children of the impl block may register hidden types for an impl Trait opaque type in an associated type. Nested syntactic items within those items may not do so. As with RPIT, closures and async blocks, which are in some sense items but are not syntactic ones and which share the generics and where clauses of their parent, may register hidden types.

Coherence rule

Coherence checking is the process by which we ensure that no two trait impls may overlap. To decide that, we must have a rule for whether any two types may be equal, for the purposes of coherence.

During coherence checking, when an opaque type is related with another type we assume that the two types may be equal unless that other type does not fulfill any of the item bounds of the opaque.

Design principles

The design described in this document for ATPIT adheres to the following principles.

Convenience and minimal overhead for the common cases

We believe that impl Trait syntax in return position and in associated type position is for convenience and should have minimal overhead to use.

The design proposed for stabilization here solves common and important use cases well, conveniently, and with minimal overhead, similar to RPIT, by e.g. leaning on the syntactic reachability rule. Other use cases may prefer to wait for full type alias impl Trait.

Local reasoning

We believe that it should be easy to determine whether an item may and must register a hidden type of an impl Trait opaque type.

In certain edge cases, whether or not an item may register a hidden type for an opaque can affect method selection and type inference. It should therefore be straightforward for the user to look syntactically at the impl block only to determine whether or not an item may register a hidden type for any opaque. This is what the syntactic reachability rule achieves.

Crisp behavior

We believe that the behavior of impl Trait should be very crisp; if an item may register the hidden type for an opaque, then within that item the type should act exactly like an inference variable (existential quantification). If it cannot, then within that item the type should act like a generic type (universal quantification).

If items were allowed to register hidden types without being required to do so, then it is believed to be either difficult or impossible to maintain this kind of crispness in all circumstances. Consequently, this design adopts the "may define == must define" rule to preserve this crisp behavior.

Motivation

We long ago stabilized async fn and async { .. } blocks as these make writing async Rust more pleasant than having to implement Future everywhere by hand.

However, there's been a lingering problem with this story. The type of the futures returned by async fn and async { .. } blocks cannot be named, and we often need to name types to do useful things in Rust. This means that we can't use async everywhere that we might want to use it, and it means that if our dependencies do use it, that can create problems for us that we can't fix ourselves.

It's for this reason that using RPIT in public APIs has long been considered an anti-pattern. Using the recently-stabilized RPITIT and AFIT in public APIs carries these same pitfalls while adding a new one: the inability for callers to set bounds on these types.

It is these problems, in the context of interfaces defined as traits, that ATPIT addresses.

Using async in more places

Today, if we want to implement IntoFuture for a type so that it can be awaited using .await, we have no choice but to implement Future::poll for some wrapper type by hand so that it can be named in the associated type of IntoFuture.

With ATPIT, for the first time, we can use async { .. } blocks to implement IntoFuture. E.g.:

use core::future::{Future, IntoFuture};

struct AlwaysAsync42;
impl IntoFuture for AlwaysAsync42 {
    type Output = impl Iterator<Item = impl Fn() -> u8>;
    type IntoFuture = impl Future<Output = Self::Output>;
    fn into_future(self) -> Self::IntoFuture { async {
        core::iter::repeat(|| 42)
    }}
}

Naming types, expressing bounds, object-safety, and precise capturing

If someone were writing a Service-like trait today, now that AFIT and RPITIT are stable, that person may think to write it as follows so that async blocks or async fn could be used in the impl:

#![allow(async_fn_in_trait)]

pub trait Service {
    type Response;
    async fn call(&self) -> Self::Response;
}

However, as compared with using associated types, that would create four problems for users of the trait:

  1. The trait would now not be object-safe.
  2. The type of the returned Future can't be named so as e.g. to store it in a struct.
  3. The type of the returned Future can't be named so as to set bounds on it, e.g. to require a Send, FusedFuture, or TryFuture bound.
  4. The type of the returned Future captures a lifetime we may not want to capture.

In the future, there may be other and better solutions to some of these problems (those solutions may in fact desugar essentially to ATPIT). But today, without ATPIT, trait authors face a dilemma. They must either accept all of these drawbacks or, alternatively, must accept that implementors of the trait will not be able to use async blocks and will have to write manual implementations of Future.

ATPIT offers us a way out of this dilemma. Trait authors can allow implementors to use async blocks (and conceivably, in the future, async fn) to implement the trait while preserving the object safety of the trait, allow users to name the type so as to store it and set bounds, and express precise capturing of type and lifetime generic parameters.

Related issues

Open items

"Must define before use"

  • Opaque types in Rust today, including stable RPIT, allow themselves to be used opaquely in a body before a hidden type is registered for the opaque. There is a proposal to reject this (tracked in #117866). This is orthogonal to ATPIT except to the degree that ATPIT would provide new ways for people to write this sort of code. We're exploring this restriction in parallel with this proposed stabilization. Regardless of the FCP completing on this issue, we'll ensure that restriction is first merged or otherwise disposed of before we merge this PR.

Add Projection/NormalizesTo goals for opaque types

  • Add Projection/NormalizesTo goals for opaque types to fix the issue raised here. Even though the example linked there results in an ICE, the ICE only happens when trying to exploit the unsoundness; the overlapping impls are simply accepted incorrectly. There is no known generally-applicable way to exploit this into actual unsoundness, but we treat accepting overlapping impls as unsound by itself. We'll ensure this is handled before merging, regardless of completion of the FCP.

Update the Rust Reference

  • Regardless of the FCP completing on this issue, we'll ensure that there is an accepted PR for updating the Rust reference before merging this PR.

Acknowledgments

The stabilization of ATPIT would not have been possible without, in particular, the ongoing and outstanding work of @oli-obk, who has been quietly pushing forward on all aspects of type alias impl Trait for years. Thanks are also due, in no small part, to @compiler-errors for pushing forward both on this work directly and on critical foundations which have made this work possible. Similarly, we can't say enough positive things about the work that @lcnr has been and is doing on the new trait solver; that work has shaped this proposal and is also what gives us confidence about the ability to support and extend this feature into the long term.

Separately, the author of this stabilization report thanks @oli-obk, @compiler-errors, @tmandry, and @nikomatsakis for their personal support on this work.

Thanks are due to the types team generally for helping develop the "Mini-TAIT" proposal that preceded this work. And thanks are of course due to the authors of RFC 1522, RFC 1951, RFC 2071, and RFC 2515, @Kimundi, @aturon, @cramertj, and @varkor, and to all those who contributed usefully to those designs and discussions.

Thanks to @nikomatsakis for setting out the design principles articulated in this document, and to @tmandry, @oli-obk, and @compiler-errors for reviewing drafts. All errors and omissions remain those of the author alone.

Footnotes

  1. ADTs with generic parameters, non-unit tuples, arrays, slices, pointers, references, function pointers, and impl Trait and dyn Trait types with generic parameters or associated types are type constructors. Traits with generic parameters or associated types, including the Fn* traits, are bounds constructors. When we say, "generic type arguments" above, we mean types that are used to parameterize these type constructors or bounds constructors, including for associated types. When we collect generic type arguments, we do so syntactically (i.e. without normalization and looking only at what is written) and recursively (i.e. within arbitrary levels of syntactic composition).

  2. There are existing restrictions on where an RPIT impl Trait may appear within a type and within a signature. Those restrictions remain and are not being changed by this stabilization.

@rustbot
Copy link
Collaborator

rustbot commented Feb 6, 2024

r? @davidtwco

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 6, 2024
@traviscross
Copy link
Contributor Author

r? @oli-obk

@rustbot rustbot assigned oli-obk and unassigned davidtwco Feb 6, 2024
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@traviscross traviscross force-pushed the TC/stabilize-atpit branch 2 times, most recently from f60e854 to f029558 Compare February 6, 2024 16:24
@rust-log-analyzer

This comment has been minimized.

@traviscross traviscross marked this pull request as ready for review February 7, 2024 05:42
@rustbot
Copy link
Collaborator

rustbot commented Feb 7, 2024

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@traviscross traviscross added T-lang Relevant to the language team, which will review and decide on the PR/issue. A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. I-lang-nominated Nominated for discussion during a lang team meeting. F-impl_trait_in_assoc_type `#![feature(impl_trait_in_assoc_type)]` and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 7, 2024
@nikomatsakis
Copy link
Contributor

nikomatsakis commented Feb 7, 2024

@rfcbot fcp merge

This is a great step forward! Super excited to see this. I'm happy we included the design axioms. Even if I drafted them initially, I had forgotten them, so it was nice to re-read them.

@rfcbot
Copy link

rfcbot commented Feb 7, 2024

Team member @nikomatsakis has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Feb 7, 2024
@ehuss
Copy link
Contributor

ehuss commented Feb 7, 2024

Can we please get a documentation PR approved before merging?

@tmandry
Copy link
Member

tmandry commented Feb 7, 2024

Thanks for your work on this, @traviscross, and everyone who was mentioned in the stabilization report. I'm excited to see this move forward.

@rfcbot reviewed

@oli-obk
Copy link
Contributor

oli-obk commented Feb 7, 2024

edit: rfcbot doesn't listen to me on T-lang FCPs ☹️

Actually landing this is also blocked on switching opaque type inference to an eager scheme (see https://rust-lang.zulipchat.com/#narrow/stream/315482-t-compiler.2Fetc.2Fopaque-types/topic/eager.20inference.20replacement.20for.20opaques/near/419219306)

added blockers as todo items to the main post. The opaque type inference change is a T-types implementation detail that almost never affects any code, but in the edge cases where it does, we need it to be consistent between our two solvers and it's also just a simpler strategy. So the FCP can continue, we'll delay actual stabilization if necessary.

@RalfJung
Copy link
Member

RalfJung commented Feb 7, 2024

Any item that is not allowed and required to register a hidden type for the opaque witnesses that opaque type as completely opaque.

I'm afraid I cannot parse this sentence. Where is the verb? Is it "witnesses", and there's a "type" missing before it? This could benefit from a rephrasing that puts the verb closer to the beginning of the sentence. :)

It's also confusing to have this before explaining the rules for which items are allowed/required to register a hidden type.

@RalfJung
Copy link
Member

RalfJung commented Feb 7, 2024

To determine the set of impl Trait opaque types for which an item may and must register a hidden type, we first collect from the signature (including from any where clauses) all types without normalization. [...]

I'm afraid I find this definition pretty unclear. What exactly are "all types"? If the signature is fn((Vec<T>, i32)), is that just (Vec<T>, i32) or also i32 and T?

For each of those types, their substitutions (i.e. generic arguments), and associated type arguments to impl Trait<..> and dyn Trait<..> types, we:

For an arbitrary Rust type T, what are its "generic arguments"? This concept only makes sense when T is an ADT or a type alias, I think. What about other types, like arrays, tuples?

@BoxyUwU

This comment was marked as resolved.

@oli-obk
Copy link
Contributor

oli-obk commented Mar 4, 2024

@rfcbot concern no-define-with-different-lifetimes

we need to land #116935 first

@aliemjay
Copy link
Member

aliemjay commented Mar 6, 2024

@rfcbot concern nested-opaques-capture-bound-lifetimes

Nested opaque types in ATPIT does not capture the lifetimes of for<_> binders. This (1) violates the new capture rules #117587, (2) silences the error higher kinded lifetime bounds on nested opaque types are not supported yet, which is necessary for forward compatibility with the new solver, and (3) is inconsistent with RPITIT

This is a bug since ATPIT is intended to follow the new lifetime capture rules.

#![feature(impl_trait_in_assoc_type)]

trait Tr<'a> { type Assoc; }
impl  Tr<'_> for () { type Assoc = (); }

// Works for ATPIT but it shouldn't
trait ATPIT {
    type Assoc;
    fn def() -> Self::Assoc;
}
impl ATPIT for u8 {
    type Assoc = impl for<'a> Tr<'a, Assoc = impl Sized>; // OK!
    fn def() -> Self::Assoc {}
}

// Doesn't compile for RPITIT as expected
trait RPITIT {
    fn def() -> impl for<'a> Tr<'a, Assoc = impl Sized>;
    //~^ ERROR higher kinded lifetime bounds on nested opaque types are not supported yet
}
impl RPITIT for u8 {
    fn def() -> impl for<'a> Tr<'a, Assoc = impl Sized> {}
    //~^ ERROR higher kinded lifetime bounds on nested opaque types are not supported yet
}

@compiler-errors
Copy link
Member

@aliemjay: I just filed a bug for this coincidentally. #122093. I'll fix it by the end of the day.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 8, 2024
…ything, r=oli-obk

Make TAITs and ATPITs capture late-bound lifetimes in scope

This generalizes the behavior that RPITs have, where they duplicate their in-scope lifetimes so that they will always *reify* late-bound lifetimes that they capture. This allows TAITs and ATPITs to properly error when they capture in-scope late-bound lifetimes.

r? `@oli-obk` cc `@aliemjay`

Fixes rust-lang#122093 and therefore rust-lang#120700 (comment)
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 8, 2024
Rollup merge of rust-lang#122103 - compiler-errors:taits-capture-everything, r=oli-obk

Make TAITs and ATPITs capture late-bound lifetimes in scope

This generalizes the behavior that RPITs have, where they duplicate their in-scope lifetimes so that they will always *reify* late-bound lifetimes that they capture. This allows TAITs and ATPITs to properly error when they capture in-scope late-bound lifetimes.

r? `@oli-obk` cc `@aliemjay`

Fixes rust-lang#122093 and therefore rust-lang#120700 (comment)
@oli-obk
Copy link
Contributor

oli-obk commented Mar 11, 2024

@aliemjay #122103 has landed

@aliemjay
Copy link
Member

@rfcbot resolve nested-opaques-capture-bound-lifetimes

@traviscross
Copy link
Contributor Author

@rustbot labels -I-lang-nominated

At this point, this is waiting on some final work to be completed. There's nothing in particular left for T-lang here, so let's remove the nomination.

@rustbot rustbot removed the I-lang-nominated Nominated for discussion during a lang team meeting. label Mar 20, 2024
@aliemjay
Copy link
Member

@rfcbot concern may-define-implies-must-define

According to the rule "may define == must define", this code should be rejected as fn take may and must define the opaque but it does not.

#![feature(impl_trait_in_assoc_type)]

pub trait Trait {
    type Assoc;
    fn give() -> Self::Assoc;
    fn take(_: Self::Assoc);
}

impl Trait for () {
    type Assoc = impl Sized;
    fn give() -> Self::Assoc {}
    fn take(_: Self::Assoc) {}
}

@Nemo157
Copy link
Member

Nemo157 commented Aug 15, 2024

rfcbot concern must-define-before-use

Opaque types in Rust today, including stable RPIT, allow themselves to be used opaquely in a body before a hidden type is registered for the opaque. There is a proposal to reject this (tracked in #117866). This is orthogonal to ATPIT except to the degree that ATPIT would provide new ways for people to write this sort of code. We're exploring this restriction in parallel with this proposed stabilization. Regardless of the FCP completing on this issue, we'll ensure that restriction is first merged or otherwise disposed of before we merge this PR.

The linked issue has been unnominated with the comment "Given that we found other paths forward", is it still relevant for the concern here (or are there other details to link to for it)?


rfcbot concern no-define-with-different-lifetimes

we need to land #116935 first

This has landed


rfcbot concern may-define-implies-must-define

According to the rule "may define == must define", this code should be rejected as fn take may and must define the opaque but it does not.

This code now produces an error:

error: item does not constrain `<() as Trait>::Assoc::{opaque#0}`, but has it in its signature
  --> src/lib.rs:12:8
   |
12 |     fn take(_: Self::Assoc) {}
   |        ^^^^
   |
   = note: consider moving the opaque type's declaration and defining uses into a separate module
note: this opaque type is in the signature
  --> src/lib.rs:10:18
   |
10 |     type Assoc = impl Sized;
   |                  ^^^^^^^^^^

@rakshith-ravi
Copy link
Contributor

Hi @nikomatsakis! Is there anything we can do to help push this forward?

@jkarneges

This comment was marked as off-topic.

@slanterns

This comment was marked as resolved.

@jkarneges

This comment was marked as resolved.

@RalfJung

This comment was marked as off-topic.

@AlseinX

This comment has been minimized.

@compiler-errors
Copy link
Member

Hold y'all's horses. It's not in FCP because the concerns have not been unblocked. Several of the people leading this work are currently away.

These concerns are not necessarily unblocked just because other people have commented that some associated PRs are landed--this work is incredibly tied up into the new trait solver, so until we guarantee that the new trait solver will not be jeopardized by inference behavior that we will not be able to replicate on it, this stabilization PR remains blocked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. F-impl_trait_in_assoc_type `#![feature(impl_trait_in_assoc_type)]` proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.