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

TAIT defining scope options #107645

Open
oli-obk opened this issue Feb 3, 2023 · 117 comments
Open

TAIT defining scope options #107645

oli-obk opened this issue Feb 3, 2023 · 117 comments
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-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` finished-final-comment-period The final comment period is finished for this PR / Issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Feb 3, 2023

during the stabilization of TAITs (type alias impl trait) in #63063 (comment) a concern was raised: it's not obvious for the compiler (and IDEs), which items' bodies are allowed to register hidden types for TAITs. For RPIT (return position impl trait) it was obvious: the body of the function in whose return type the impl Trait was in. For TAITs the status quo is point 1 in the list of options below. This may require some interesting module juggling to avoid cycle errors that can occur due to revealing the hidden type to prove Send or Sync bounds, and that revealing again looking at the item to look for things registering the hidden type (see example and passing example).

The possible schemes we know about currently are:

  1. any item that is (recursively) within the parent of the TAIT is allowed to register the hidden type
  2. any item that follows the rules of 1. and additionally has the TAIT in its signature or where bounds is allowed to register the hidden type. this change has a prospective impl at
  3. same as 2. but not permitting where bounds to register hidden types
  4. any item in the defining scope can register hidden types, but we error out if the TAIT is not mentioned in the signature or where bounds
  5. same as 4. but not permitting where bounds to register hidden types
    • forward compatible with choosing 4. later

Ruled out schemes:

  1. any item that follows the rules of 1. and has a #[defines(NameOfTheTypeAlias)] attribute is allowed to register the hidden type
    • probably undesirable due to the verbosity in "obvious" cases (like using a TAIT in function return position)
  2. only allow explicitly registering hidden types with a magic libcore function that syntactically mentions the TAIT by its path. This will still require looking at the body, but we don't need to typecheck the body, just do name resolution on it.
    • this option is just listed for completeness, it's the worst option :)

I am nominating this issue for T-lang to discuss which options they prefer.

@oli-obk oli-obk added A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` I-lang-nominated Nominated for discussion during a lang team meeting. labels Feb 3, 2023
@pnkfelix
Copy link
Member

Discussing T-lang triage. We want/need to resolve choice between options provided asynchronously.

After we come to some conclusion, we'll FCP merge/close this to demarcate that conclusion.

@pnkfelix
Copy link
Member

@nikomatsakis

This comment was marked as outdated.

@nikomatsakis nikomatsakis added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Mar 14, 2023
@nikomatsakis
Copy link
Contributor

@rfcbot fcp merge

I propose that we accept #107809. It implements a conservative path forward. Basically any function that constraints a TAIT but doesn't list the TAIT in its arguments/return type is a hard error, giving us room to change the behavior in the future.

Final behavior as I understand it

  • A TAIT has a defining scope that corresponds to the enclosing module or item.
  • A defining use for a TAIT is any item that (a) is within the defining scope and (b) contains a function that lists the TAIT in the argument or return types, either before or after normalization (*see edge case below).
  • Within the defining scope, an item is called constraining if it puts constraints on the value of the TAIT. i.e., for the item to type check, the hidden type of the TAIT must have a particular value. This could occur because of a let (e.g., let x: TAIT = 22_u32), a return (e.g., return 22_u32 in a function whose return type is TAIT), or in other ways.
  • Any constraining item within the defining scope that is not a defining use is a hard error. This means we can later opt to allow such a use; or to allow it with an annotation of some kind; or to make other such changes.
  • All defining uses must fully infer the hidden type of the TAIT and must infer the same type for the TAIT.
  • WIthin the defining scope, TAITs must always be given generic arguments (e.g., fn foo<T>() -> TAIT<T> and not fn foo() -> TAIT<u32>). This ensures inference is tractable and well-defined.

Current bugs and limitations (forwards compatible to change)

  • Within the defining scope, attempts to check whether TAIT implements an auto-trait will yield a cycle error unless the auto-trait is listed in the TAIT's bounds. This is suboptimal, but the ideal fix is unclear.
  • A function that has an argument which is an associated type referencing a TAIT (e.g. <TAIT as SomeTrait>::SomeItem) ought to be considered a defining use. However, in the compiler today, if that associated type can be normalized, and the normalized form does not reference the TAIT, the function is not. This can only cause more errors.

@rustbot labels -I-lang-nominated

@rfcbot
Copy link

rfcbot commented Mar 14, 2023

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

Concerns:

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.

@rustbot rustbot removed the I-lang-nominated Nominated for discussion during a lang team meeting. label Mar 14, 2023
@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 Mar 14, 2023
@tmandry
Copy link
Member

tmandry commented Mar 14, 2023

@rfcbot concern why-not-just-the-return-type

Can we restrict to only the return type, instead of the return type and arguments? Are there major use cases that wouldn't be satisfied by this?

cc @oli-obk

@oli-obk
Copy link
Contributor Author

oli-obk commented Mar 14, 2023

Not that I know of, though it may feel like a very artificial limitation to users

@RalfJung
Copy link
Member

We allow impl Trait consistently in argument and return position, so why should TAIT not work in both? Is the concern the polarity of quantification which is common between TAIT and return types but not argument types?

@nikomatsakis
Copy link
Contributor

I am fine with adding a restriction to return position. I think it covers the major use cases (though perhaps @Dirbaio may wish to weigh in? I recall Embassy making creative use of TAITs).

I also like the story of "you can now take an impl Trait that you had previously written in return position and pull it into a type alias-- that is true for return type. For argument position it's less true, as the semantics are different.

But to me the biggest detail is that we can easily allow more positions, but it's hard to take them back.

@RalfJung
Copy link
Member

RalfJung commented Mar 14, 2023

I also like the story of "you can now take an impl Trait that you had previously written in return position and pull it into a type alias-- that is true for return type. For argument position it's less true, as the semantics are different.

It's not entirely true, is it? In fn foo<T>(x: T) -> impl Trait, the chosen underlying type can depend on T, but if it pulled out then it cannot since it is quantified "outside" T (∀T ∃U vs ∃U ∀T). This is a consequence of the same shifting around of the quantifier that also makes it not work for arguments (except for arguments the polarity of the ∃ changes making it a bigger difference).

@joshtriplett
Copy link
Member

I would prefer to allow it anywhere in the argument or return types. What would be the rationale for limiting it to just return types?

@nikomatsakis
Copy link
Contributor

@RalfJung

It's not entirely true, is it? In fn foo<T>(x: T) -> impl Trait, the chosen underlying type can depend on T, but if it pulled out then it cannot since it is quantified "outside" T (∀T ∃U vs ∃U ∀T).

it can, but you have to write

type Foo<T> = impl Trait;

fn foo<T>(x: T) -> Foo<T> { ..}

@Dirbaio
Copy link
Contributor

Dirbaio commented Mar 14, 2023

in Embassy, the user writes this:

#[embassy_executor::task]
async fn run() {
    // stuff
}

which gets expanded to:

async fn __run1_task() {
    // stuff
}

fn run1() -> ::embassy_executor::SpawnToken<impl Sized> {
    type Fut = impl ::core::future::Future + 'static;
    static POOL: ::embassy_executor::raw::TaskPool<Fut, 1usize> = ::embassy_executor::raw::TaskPool::new();

    // defining use here. Signature for `_spawn_async_fn` is:
    // impl<F: Future + 'static, const N: usize> TaskPool<F, N> {
    //     fn _spawn_async_fn<FutFn>(&'static self, future: FutFn) -> SpawnToken<impl Sized>
    //          where FutFn: FnOnce() -> F;
    // }
    // so it constrains `Fut` to be the future for `__run1_task()`.
    unsafe { POOL._spawn_async_fn(move || __run1_task()) }
}

Honestly, I'm having a very hard time figuring out whether this'll be allowed in the new rules from reading #107645 (comment) ... Here's my thought process:

A TAIT has a defining scope that corresponds to the enclosing module or item.

Okay, so the defining scope is run1's body.

A defining use for a TAIT is any item that (a) is within the defining scope and (b) contains a function that lists the TAIT in the argument or return types, either before or after normalization (*see edge case below).

The defining use here is an expression. Are expressions "items"? They aren't, according to this?

Either way, even if expressions are items, that expression wouldn't be a "defining use" because it doesn't contain such a function.

(btw, "item contains a function that lists the TAIT" is very confusing. This means the item can be an impl or a mod that contains the function, it doesn't need to be a function itself? The function can be nested 100 mods deep, or does it have to be an immediate child?)

Within the defining scope, an item is called constraining if it puts constraints on the value of the TAIT. i.e., for the item to type check, the hidden type of the TAIT must have a particular value. This could occur because of a let (e.g., let x: TAIT = 22_u32), a return (e.g., return 22_u32 in a function whose return type is TAIT), or in other ways.

The expression is constraining, but if it's not an item then it's not a "constraining item"...???

Any constraining item within the defining scope that is not a defining use is a hard error. This means we can later opt to allow such a use; or to allow it with an annotation of some kind; or to make other such changes.

Again, is an expression a "constraining item"? if yes, then I guess Embassy's use is no longer allowed?
If it's not, then it's allowed, because it's not a "constraining item" therefore it doesn't trigger an error.

All defining uses must fully infer the hidden type of the TAIT and must infer the same type for the TAIT.
WIthin the defining scope, TAITs must always be given generic arguments (e.g., fn foo() -> TAIT and not fn foo() -> TAIT). This ensures inference is tractable and well-defined.

This seems the same as before, so we should be good here.


The previous rules around defining scopes (the ones in nightly currently) were already confusing enough, but the new ones are way way worse. They seem incredibly complex to me. I consider myself an experienced Rust user, and yet I can't figure out how to apply them to this case.

IMO Rust should prioritize user-friendliness over IDE/compiler-friendliness. This seems the wrong tradeoff to me.

@Dirbaio
Copy link
Contributor

Dirbaio commented Mar 14, 2023

Why was #[defines()] ruled out? That seems the most straightforward approach to me.

The previous "defining scope is the parent item of the TAIT" rule already felt very artificial to me. The new "defining scope is any function that's child of the parent item of the TAIT, that mentions the TAIT in the return type after normalization" rule feels even more artificial.

While with #[defines()], it's all very natural and straightforward:

  • Each TAIT must have one (or more..?) #[defines()].
  • The #[defines()] can be anywhere within the current crate. No more "parent item".
  • It's immediately obvious why type inference behaves differently inside and outside the #[defines()].

It's slightly more verbose, sure. I don't think that's a problem. TAIT is a rather advanced feature that will be used rarely, and mostly in library/platform code, not end-user code. It interacts in a very complex way with type inference, having explicit #[defines] helps tame the complexity a bit.

@Nemo157
Copy link
Member

Nemo157 commented Mar 14, 2023

TAIT is a rather advanced feature that will be used rarely, and mostly in library/platform code, not end-user code.

I disagree, in previous experiments I've found TAIT to be really helpful when doing trait implementations in applications, things like impl IntoIterator { type IntoIter = impl Iterator<Item = Self::Item>; ... }. Those can come up often in "end-user" code (tbh I hate that kind of distinction too, end-users should be writing their own "mini-libraries" for their usecases so everyone is a library dev).

@Dirbaio
Copy link
Contributor

Dirbaio commented Mar 14, 2023

Okay, I agree the "user vs library code" is not a very strong argument. The other arguments still stand: it's not that verbose, and it massively reduces complexity and implicit stuff.

This is not that verbose:

impl IntoIterator for Foo { 
    type IntoIter = impl Iterator<Item = Self::Item>;

    #{defines(Self::IntoIter)}
    fn into_iter(self) -> Self::IntoIter {
        ...
    }
}

perhaps it can be made less verbose too, though I don't like the idea having 2 syntaxes for the same thing: fn into_iter(self) -> #[defines] Self::IntoIter {..}

@nikomatsakis
Copy link
Contributor

@Dirbaio that case may not be covered by the rules exactly as I wrote them; I think it's a reasonable extension however to include the types of statics and constants. @oli-obk, comment?

@Dirbaio
Copy link
Contributor

Dirbaio commented Mar 15, 2023

yeah, but why? Why do we need such complicated rules?

@nikomatsakis
Copy link
Contributor

Regarding #[defines], I see the appeal for TAIT and I'm not dead opposed, though there is some "weirdness" (see below). It seems mildly inconsistent with other (existential) uses of impl Trait, though, which all have an implicit enclosing scope of the surrounding item.

Example: fn items. Function items of course have a a defining scope of the function (and this is stable):

fn foo() -> impl Bar { } // defined by this function

Example: impl assoc types. This stabilization includes (I believe) impl Baz in impl positions, which have a defining scope of the impl (particularly, other methods in the impl):

impl Foo for Bar {
    type IntoIter = impl Baz; // defined by methods in this impl (which return this assoc type)
}

Example: const or static. Embassy relies on impl Trait in the type of a consts/statics, which I think again have a very natural scope and should be allowed:

const Foo: impl Debug = 22;

Example: TAIT. The question then is what happens when we extend to a TAIT like the following

type Foo = impl Debug;

The current proposal continues with "defining scope is the enclosing item", and in this case the rule is this impl Trait desugars to an opaque type defined by "functions that return or accept Foo in the enclosing item" (just as with an impl).

Including defines would break the pattern. I think there's a decent case for that, basically the argument is that top-level items are different. These are the things that can be "used" and imported and so forth. Potentially true, although the idea of having #[defines] stretch across crates makes me mildly uncomfortable, and I'd hold off on that.

Alternative: defines for TAIT. The proposal would then be: if you define a TAIT (as opposed to an impl Trait embedded in an item, as shown above), you must also have #[defines(path)] where path is a path that resolves to the type alias. This must appear inside the module that declares the TAIT (transitively). Items that don't have #[defines] use the TAIT opaquely.

Multiple opaque types. It's worth pointing out that there is not a 1:1 relationship between a type name like Foo and an opaque type. You can have:

type Bar = (impl Debug, impl Debug);

but I suppose that saying #[defines(Bar)] would imply that you define all opaque types defined within.

One consideration, which I don't think is a particularly big deal, is that you can't extract a type-alias for an impl Trait return type without also adding #[defines]:

fn foo() -> impl Debug { }

// becomes

type Foo = impl Debug;

#[defines(Foo)]
fn foo() -> Foo { ]

This doesn't seem like a big deal because extracting impl Trait into a TAIT wouldn't work reliably anyway; moreover, this actually increases fidelity in some ways, because the above is exactly equivalent to -> impl Debug.

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Mar 15, 2023

@Dirbaio see above. I appreciate the pushback. I could get behind a version like so.

Sugary cases: fns/impls/consts/statics

  • impl trait within the return type of a fn creates an opaque type whose hidden type is inferred by the type returned by that fn
  • impl trait within the type of a const or static creates an opaque type whose hidden type is inferred by the type of that initializer
  • impl trait in the value of an associated type in an impl creates an opaque type whose hidden type is inferred by other members of the impl matching the above rules

Explicit case: type aliases.

  • impl trait in a type alias P creates an opaque type whose hidden type is inferred by items that are annotated with #[defines(P)] -- possibly restricted to the module defining P, although I agree that's not necessary

I like that it is possible to precisely desugar all the "sugary" cases to a TAIT that precisely matches their semantics. I also like that behavior with auto traits and other things is much easier to explain.

@tmandry
Copy link
Member

tmandry commented Nov 1, 2023

After much helpful discussion with @traviscross and @compiler-errors, I'm resolving my outstanding concerns.

@rfcbot resolve nested-modules-can-always-define-but-nested-functions-cannot

I'm not too concerned about this inconsistency, and only ever wanted some reason it should be this way. I believe the reason was that tools such as rust-analyzer would want to be able to skip parsing function bodies. On the other hand, it seems likely that we will change it anyway, given its interactions with rules recently proposed by @rust-lang/types.

@rfcbot resolve encapsulation-is-too-powerful

My reason for resolving this concern is the combination of the following:

  1. We must decide, from the point of stabilization, which functions are allowed to constrain. This is because changing the rule would affect type inference in those functions, and cause existing code to break.1 If "allowed to constrain" is based on an implicit rule like the signature restriction, it has to be fully defined from the beginning, though it could later be expanded to include more cases using explicit syntax like #[defines].
  2. There's a case to be made that the signature restriction covers a large portion of use cases and is a reasonable compromise between explicitness and ease of use; that is in large part what the doc that prompted this FCP is about, and all the lang team members present in the meeting checked their boxes.
  3. There's a case to be made that not allowing "constraining through encapsulation" would be too annoying. If we can't change our minds to allow it in the future, we might as well allow it now.
  4. While I would prefer to use #[defines] to resolve this, it seems unlikely that we will have #[defines] soon. The reason is that there are significant open design questions around the syntax, and that there are implementation challenges that prevent us from implementing it today. This presents a substantial risk to our ability to ship TAITs in 2024, which in addition to being desirable on its own, is required to ship Lifetime Capture Rules 2024 rfcs#3498.
  5. Allowing implicit constraining via the signature restriction with encapsulation is forward-compatible with requiring #[defines] later on signatures of functions that constrain via encapsulation (or all functions that constrain, for that matter). This could be done as a lint, or even a hard requirement over an edition. On the other hand, if we required #[defines] now, it would not be forward-compatible with relaxing to the signature restriction later, for the reason described in (1).2 It is also forward-compatible with allowing functions to constrain from outside the defining module with #[defines].

I feel a bit weird about basing a decision on so many factors I consider necessary for that decision, but it seems like this is just a complex problem space. I'm laying them out here so I (and others) can refer back to them.

With all that said, we do need another t-lang meeting before stabilizing TAIT. I've asked for the future possibility of #[defines] to be included in that meeting so the full team understands the problem space around it at a high level.

Footnotes

  1. @rust-lang/types determined that "constraining or not" must be an input to type inference to make the feature work as intended.

  2. There is another maximally forward-compatible option we ruled out that does not require lints or an edition, which would be to require that every function that meets the signature restriction also has #[defines]. This would be required even if the user did not need or want the function to define the hidden type. It would allow us to defer the decision of whether to require #[defines] always, or allow functions which pass the signature restriction to constrain implicitly. On the other hand, in the meantime it would be annoying, confusing, and loud (syntax-wise), and it suffers from the problem in point (4) above – not implementable in a reasonable time frame.

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Nov 1, 2023
@rfcbot
Copy link

rfcbot commented Nov 1, 2023

🔔 This is now entering its final comment period, as per the review above. 🔔

@Dirbaio
Copy link
Contributor

Dirbaio commented Nov 1, 2023

There's a case to be made that the signature restriction covers a large portion of use cases and is a reasonable compromise between explicitness and ease of use

This is not true at all. We did a survey of code in the wild a while ago on Zulip with @traviscross, and the result was that the "parent mod + signature restriction" works about 50% of the time, and requires refactors, dummy modules or dummy functions otherwise.

i believe the proposed "functions meeting the signature restriction MUST constrain" change will reduce the percentage further.

works: user adds TAIT to existing code, the restrictions pass, everything compiles.

fails: TAIT doesn't work out of the box, user must do dummy modules/functions or move code as a workaround.

Note some of these are from before the signature restriction landed on nightly, so just the "parent mod" restriction was already causing problems.

Also note there is a very strong "survivorship bias" in searching GitHub. When TAIT doesn't work, users are likely to fall back to using Box<dyn Trait>, so the data point about TAIT's failure doesn't end up on GitHub at all.

This presents a substantial risk to our ability to ship TAITs in 2024, which in addition to being desirable on its own, is required to ship rust-lang/rfcs#3498.

TAIT doesn't have to block the RPIT capture rules. There's the impl<'a, T> Trait syntax as an alternative. (which is something I believe should be done, forcing desugaring to TAITs is not very ergonomic)

@traviscross
Copy link
Contributor

traviscross commented Nov 1, 2023

Thanks @Dirbaio for taking the time to provide a counterpoint.

Regarding the examples you helpfully provided in the second list, I observe that 4 out of the 6 on the list I can review are due to cycle errors or frustration with cycle errors.1

We all agree that spurious cycle errors are annoying and have been a pain point, and we're eliminating those as part of this plan. Under this plan, with the new trait solver, there will be zero spurious cycle errors regardless of how the code is factored. The details of the plan have been carefully chosen to ensure this.

As for the remaining two examples, Example 5 would either need the opaque added in a where clause on main (the simplest fix) or it would need a helper function. Since in this example the type can actually be named (it's using TAIT instead of the newtype pattern), that could be done without any meaningful refactoring using the "wrap/unwrap" pattern2:

type Led = impl OutputPin;
fn to_led(x: Output) -> Led { x }
// ...
let led = to_led(Output::new(..));

In some contexts (perhaps not this one), it would be reasonable for such an Output type to grow a method that returned the named opaque type, in the style of the Builder pattern.

The way that Embassy uses TAIT as an alternative to the newtype pattern is not really what people had in mind when designing TAIT. It's a surprising benefit that it does in fact work for this use-case also with some simple patterns such as the Builder pattern or the "wrap/unwrap" pattern demonstrated above.

Regarding Example 7, that one is difficult to analyze. That's deep in some code-generating code for RTIC. It wouldn't surprise me if code-generating code had to more consciously accommodate the restrictions, as such code often has to do.

Again, thanks for the interesting examples and for providing a counterpoint.

Footnotes

  1. Example 1 was in the Rust compiler itself and was due to a spurious cycle error. The workaround was to add an explicit Send bound. Example 2 was also due to cycle errors as you noted. In Example 3, the comment on the code notes that it's due to the cycle errors reported in Cycle detected when processing existential type #55997. In Example 5, the person mentions cycle errors as a key frustration twice in a short note. For Example 6, that link does not load for me, so I excluded it from the count.

  2. In the general case, the pattern looks like this:

    struct MyType;
    type Tait = impl Sized;
    fn wrap(x: MyType) -> Tait { x }
    fn unwrap(x: Tait) -> MyType { x }
    

@Dirbaio
Copy link
Contributor

Dirbaio commented Nov 1, 2023

We all agree that spurious cycle errors are annoying and have been a pain point, and we're eliminating those as part of this plan. Under this plan, with the new trait solver, there will be zero spurious cycle errors regardless of how the code is factored. The details of the plan have been carefully chosen to ensure this.

The proposed solution to eliminate the cycle errors as far as I understand from the meeting minutes is "if function meets the parent mod + signature restrictions, it MUST constrain the opaque" (vs currently it MAY constrain).

Cycle errors currently happen when a function

  • passes the restrictions
  • doesn't constrain the opaque, and
  • tries to inspect auto traits of it.

With that proposed solution, such code will still be rejected (because the fn passes the restrictions but doesn't constrain). The change does improve the error message, which is nice, but doesn't make these cases work. The user still has to refactor their code, like moving that function outside the TAIT's parent module, or make a dummy module to reduce the defining scope.

Example 5 would either need the opaque added in a where clause on main (the simplest fix) or

Dummy where clauses are a bad workaround for a few reasons:

  • main can't have where clauses (neither on std Rust, nor on Embassy)
  • Where clauses are part of the public API. They show up in rustdoc etc. By being forced to mention the TAIT there, you're leaking implementation details of the function. Also, if the function is public and the TAIT is private, it gives you a spurious warning. See playground

or it would need a helper function. Since in this example the type can actually be named (it's using TAIT instead of the newtype pattern), that could be done without any meaningful refactoring using the "wrap/unwrap" pattern"

The whole point of using TAIT there is to avoid having to write out the full type. These workarounds require writing out the full type, so they completely defeat the point.

In that example the full type is Output<'static, PB7>. Not too bad to type by hand, but it hardcodes the pin name (PB7), which change from board to board, which is why the user wanted to let the compiler infer it. In more complex examples the types get longer: example, example.

All these examples could use TAIT if we had #[defines]. Your suggested workarounds don't work because they require writing out the full type, which is what we were trying to avoid in the first place.

The way that Embassy uses TAIT as an alternative to the newtype pattern is not really what people had in mind when designing TAIT

The use case is not "alternative to the newtype pattern". It's letting the compiler infer long complex types so I don't have to type them out by hand.

And how is that use case not intended by the original TAIT design? Quoting from the original TAIT RFCs:

RFC 2071, impl Trait existential types:

In its current form, the feature makes it possible for functions to return unnameable or complex types such as closures and iterator combinators.

That's exactly what I want to do, pass a complex type without having to write it out manually.

RFC 2515, Type Alias Impl Trait:

Allow type aliases and associated types to use impl Trait, replacing the prototype existential type as a way to declare type aliases and associated types for opaque, uniquely inferred types.

Again, that's exactly what I want to do. Declare a type alias (i.e. a simpler name) for a type while letting the compiler infer it for me, so I don't have to type it out.

These use cases I'm bringing up fit perfectly with the original goals of TAIT. Yet the current design can't make them work.

It's a surprising benefit that it does in fact work for this use-case also with some simple patterns such as the Builder pattern or the "wrap/unwrap" pattern demonstrated above.

Again: no, they don't work, because the goal of using TAIT here is avoiding writing out the full type.

@tmandry
Copy link
Member

tmandry commented Nov 1, 2023

The proposed solution to eliminate the cycle errors as far as I understand from the meeting minutes is "if function meets the parent mod + signature restrictions, it MUST constrain the opaque" (vs currently it MAY constrain).

That's correct, but I understand that this restriction can be relaxed once the new trait solver lands (or we fix the old one enough to be confident that switching to the new solver won't break existing code).

I'm wondering if it's feasible to let functions that do not constrain continue to compile under a feature flag, with the expectation that some instances of the pattern will require refactoring in the future. I think @compiler-errors or @oli-obk would know.

@traviscross
Copy link
Contributor

traviscross commented Nov 1, 2023

Thanks @Dirbaio for taking the time to make a detailed reply. I'll try to fill in some context below.

The proposed solution to eliminate the cycle errors as far as I understand... is "if function meets the parent mod + signature restrictions, it MUST constrain the opaque" (vs currently it MAY constrain).

The purpose of this restriction is to preserve forward compatibility with the new trait solver. Once the new trait solver lands, this restriction could be lifted and there would still be no spurious cycle errors.

By lifting this restriction, we would be able to accept the kind of code that you mention as being desirable to accept without refactoring.

Dummy where clauses are a bad workaround for a few reasons...

Setting aside stylistic questions about this and focusing just on feasibility (as it may be better or more idiomatic to use other patterns here in any case), there are options to address the points you raise using patterns already common in Rust.

While main cannot have a where clause, the contents of main can simply be moved to another function that main calls. People routinely use this pattern for other reasons, e.g. try_main. Alternatively, if we allow nested inner functions to constrain in the way we will propose as part of the T-types restrictions, then one could write:

type MyTait = impl Sized;
fn main() {
    fn inner() where MyTait: Any {
        let _: MyTait = ();
    }
    inner();
}

The inner function pattern is also common in Rust for many other purposes.

Using inner functions in this way would also prevent the where clause from appearing in the API documentation for the function or method. And it avoids the warning about a private type alias in a public function (playground link).

(But again, it's probably not worth debating this point too much, as other patterns are likely to be superior overall.)

The whole point of using TAIT there is to avoid having to write out the full type..... The use case is not "alternative to the newtype pattern". It's letting the compiler infer long complex types so I don't have to type them out by hand.

People can disagree about what the purpose of an RFC was, and that's OK. Whether or not the use case you're describing was the original purpose, I think it's great that we can support it with some common patterns.

However, my reading of the RFCs is a bit different than yours. The original design intent, as I understand it, was to allow naming types that currently cannot be named, and also to allow interfaces like the one Iterator to be able to return opaque types without creating problems for downstream users due to there being no way to name those types.

It wasn't, in my understanding, intended as a generalized mechanism for letting the compiler infer types so they don't have to be written out. In fact, people (including you, if I recall correctly) have previously proposed non-opaque versions of TAIT (or similar) that would better address this use case. But there has not so far been appetite for this for various reasons.

Where this distinction matters is that for the intended use cases like Iterator, passing the signature restriction is easy and automatic. But when trying to use this as a general mechanism to avoid having to write out type signatures, it is easier to run into cases where patterns must be used to facilitate this.

Again, thanks for taking the time to engage so thoughtfully with this.

@Dirbaio
Copy link
Contributor

Dirbaio commented Nov 1, 2023

the contents of main can simply be moved to another function that main calls. People routinely use this pattern for other reasons, e.g. try_main

This is just another workaround. (A workaround for issues caused by the first workaround!) Also, it should be on the user to decide if they want to do the "inner main" pattern, it shouldn't be forced on them by the language.

The original design intent, as I understand it, was to allow naming types that currently cannot be named, and also to allow interfaces like the one Iterator to be able to return opaque types without creating problems for downstream users due to there being no way to name those types.

The RFC LITERALLY says "unnameable or complex types".

It wasn't, in my understanding, intended as a generalized mechanism for letting the compiler infer types so they don't have to be written out. In fact, people (including you, if I recall correctly) have previously proposed non-opaque versions of TAIT (or similar) that would better address this use case. But there has not so far been appetite for this for various reasons.

I disagree, there is A LOT of appetite for cross-function type inference (for either unnameable, or merely "complex" types). I think the links I've provided on my previous post of people trying to do that proves this beyond any doubt.

@traviscross
Copy link
Contributor

@Dirbaio:

This is just another workaround... it should be on the user to decide if they want to do the "inner main" pattern...

We could say this about many patterns in the language. E.g., people use the inner function pattern to reduce the size of generated code due to generics. Is that actually a workaround? Maybe. We accept this in many cases as reasonable.

The RFC LITERALLY says "unnameable or complex types".

Specifically, RFC 2071 says:

This RFC proposes two expansions to Rust's impl Trait feature. impl Trait... allows functions to return types which implement a given trait, but whose concrete type remains anonymous.... In its current form, the feature makes it possible for functions to return unnameable or complex types such as closures and iterator combinators. impl Trait also allows library authors to hide the concrete type returned by a function, making it possible to change the return type later on.

As mentioned, people can disagree about the interpretation of text, and that's OK. My reading is that the authors used the word "complex" here so as to include Iterator in the intended use case as they describe in this sentence and throughout the document. These complex types can be named currently, but the intent of this RFC was to allow such interfaces to return these types opaquely without creating problems for downstream code.

I disagree, there is A LOT of appetite for cross-function type inference....

There has been limited appetite among the lang team. But it's reasonable to point out the possible presence of appetite in a larger sense, so thanks for doing that so we can distinguish these.

@Dirbaio
Copy link
Contributor

Dirbaio commented Nov 1, 2023

I'm surprised at your position that these use cases are not intended to be solved by TAIT. Why wasn't this brought up in our previous discussion, 4 months ago? (this Embassy issue is from after that discussion, but we did discuss the other examples from embedded code, and they share the same characteristics).

IMO it'd be a shame to leave these use cases unserved by TAIT, considering they are served well if we do defines.

@traviscross
Copy link
Contributor

traviscross commented Nov 1, 2023

@Dirbaio:

I'm surprised at your position that these use cases are not intended to be solved by TAIT....

My position is that the original RFCs did not intend TAIT as a generalized mechanism for letting the compiler infer types so they don't have to be written out.

....Why wasn't this brought up in our previous discussion, 4 months ago?

We did discuss this at that time. E.g.:

The distinction I'm drawing is that the original authors thought of hiding the concrete type in the context of exposing the opaque type from an API.

(We've discussed many things. It's understandable to forget such details.)

....IMO it'd be a shame to leave these use cases unserved by TAIT...

In all of the examples we've discussed, the users were able to solve the problems they set out to solve using TAIT. The biggest pain point in the past that has put an asterisk on that has been spurious cycle errors, and we're eliminating those.

@jackh726
Copy link
Member

jackh726 commented Nov 2, 2023

Hi all, please be mindful of subscribers of this issue, and to keep discussion on-topic and moving (rather than hammering on the same points over and over again).

@Dirbaio I'm sympathetic to your plea to make TAITs applicable to all the use cases people want for them out-of-the-gate. However, at the end of the day, getting TAITs stabilized to cover 50% of use cases for them without preventing future expansion to the remaining 50% is much better than delaying stabilization on building not just consensus an alternative, but implementing that alternative.

If you have new thoughts or data to share on this, I'm happy to discuss on Zulip.

@Dirbaio
Copy link
Contributor

Dirbaio commented Nov 7, 2023

I've collected all my thoughts on TAIT defining scope design in a document. It goes through the possible designs and pros and cons of explicit (defines-like) and implicit (like "parent mod + signature restriction") designs, and what data we have about their use cases.

https://hackmd.io/i-xpzf-LR7q75pSRTDVSaA

zulip thread: https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/TAIT.20defining.20scope.20implicit.20vs.20explicit

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Nov 11, 2023
@rfcbot
Copy link

rfcbot commented Nov 11, 2023

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@rfcbot rfcbot added the to-announce Announce this issue on triage meeting label Nov 11, 2023
@GoldsteinE
Copy link
Contributor

I don’t think @Dirbaio points were properly addressed? Zulip threads seems dead and I don’t see a proper argument against points in Hackmd, only clarifying questions/answers.

@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Dec 14, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 22, 2024
…mpiler-errors

Split tait and impl trait in assoc items logic

And simplify the assoc item logic where applicable.

This separation shows that it is easier to reason about impl trait in assoc items compared with TAITs. See https://rust-lang.zulipchat.com/#narrow/stream/315482-t-compiler.2Fetc.2Fopaque-types/topic/impl.20trait.20in.20associated.20type for some discussion.

The current plan is to try to stabilize impl trait in associated items before TAIT, as they do not have any issues with their defining scopes (see rust-lang#107645 for why this is not a trivial or uncontroversial topic).
fmease added a commit to fmease/rust that referenced this issue Jan 23, 2024
…mpiler-errors

Split tait and impl trait in assoc items logic

And simplify the assoc item logic where applicable.

This separation shows that it is easier to reason about impl trait in assoc items compared with TAITs. See https://rust-lang.zulipchat.com/#narrow/stream/315482-t-compiler.2Fetc.2Fopaque-types/topic/impl.20trait.20in.20associated.20type for some discussion.

The current plan is to try to stabilize impl trait in associated items before TAIT, as they do not have any issues with their defining scopes (see rust-lang#107645 for why this is not a trivial or uncontroversial topic).
fmease added a commit to fmease/rust that referenced this issue Jan 23, 2024
…mpiler-errors

Split tait and impl trait in assoc items logic

And simplify the assoc item logic where applicable.

This separation shows that it is easier to reason about impl trait in assoc items compared with TAITs. See https://rust-lang.zulipchat.com/#narrow/stream/315482-t-compiler.2Fetc.2Fopaque-types/topic/impl.20trait.20in.20associated.20type for some discussion.

The current plan is to try to stabilize impl trait in associated items before TAIT, as they do not have any issues with their defining scopes (see rust-lang#107645 for why this is not a trivial or uncontroversial topic).
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 24, 2024
Rollup merge of rust-lang#119766 - oli-obk:split_tait_and_atpit, r=compiler-errors

Split tait and impl trait in assoc items logic

And simplify the assoc item logic where applicable.

This separation shows that it is easier to reason about impl trait in assoc items compared with TAITs. See https://rust-lang.zulipchat.com/#narrow/stream/315482-t-compiler.2Fetc.2Fopaque-types/topic/impl.20trait.20in.20associated.20type for some discussion.

The current plan is to try to stabilize impl trait in associated items before TAIT, as they do not have any issues with their defining scopes (see rust-lang#107645 for why this is not a trivial or uncontroversial topic).
@BatmanAoD
Copy link
Member

The status of this issue is confusing -- what actually got approved & merged by the FCP ('m assuming this), and should this issue have been closed once the merge occurred?

@traviscross
Copy link
Contributor

The status is that we have a pending stabilization for ATPIT (#120700) that is waiting on implementation work, and that for TAIT more broadly, we're likely to do some kind of #[defines(..)]-like scheme, with the details of that still to be decided.

It's probably reasonable to leave this issue open until we close it in favor of another, e.g. perhaps one that settles those details.

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-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` finished-final-comment-period The final comment period is finished for this PR / Issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet