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

Permit impl Trait in type aliases #2515

Open
wants to merge 20 commits into
base: master
from

Conversation

Projects
None yet
@varkor
Copy link
Member

commented Aug 5, 2018

Allow impl Trait to be used in type aliases and associated traits, resolving the open question in RFC 2071 as to the concrete syntax for existential type. This makes it possible to write type aliases of the form:

type Adder = impl Fn(usize) -> usize;
// equivalent to: `existential type Adder: Fn(usize) -> usize;`

Rendered.

Thanks to @rpjohnst and @Centril for their ideas, discussion and feedback.

@Centril Centril added the T-lang label Aug 5, 2018

In addition, when documenting `impl Trait`, explanations of the feature would avoid type theoretic terminology (specifically "existential types") and prefer type inference language (if any technical description is needed at all).

## Restricting compound `impl Trait` trait aliases
The type alias syntax is more flexible than `existential type`, but for now we restrict the form to that equivalent to `existential type`. That means that, if `impl Trait` appears on the right-hand side of a type alias declaration, it must be the only type. The following compound type aliases, therefore, are initially forbidden:

This comment has been minimized.

Copy link
@Nemo157

Nemo157 Aug 5, 2018

Contributor

My biggest issue with this restriction is that it makes impl Trait inconsistent between type aliases and everywhere else (excluding the already inconsistent argument position). With existential type there was a simple rule that could be applied to impl Trait in every position except argument position: it introduces a new anonymous existential type in the current context. This rule works perfectly for return position, type declaration in bindings, type aliases, and could work for type declaration of struct/enum members if there wasn't a chance of confusion with argument-position-impl-trait.

This comment has been minimized.

Copy link
@varkor

varkor Aug 5, 2018

Author Member

I'm not quite sure I follow your point. This restriction is simply a syntactic one: it is simply intended to sidestep the question of what:

type Foo = (impl Bar, impl Bar);

means for now (because some people expressed unease at this construction in particular).

impl Trait continues to be applicable in exactly the same places as existential type: this rule simply means it can't be used in more complex scenarios than existential type yet.

This comment has been minimized.

Copy link
@Nemo157

Nemo157 Aug 5, 2018

Contributor

I mean that with existential types it's possible to have a single very simple rule for desugaring impl Trait that covers both:

type Foo = (impl Bar, impl Bar);
let foo: (impl Bar, impl Bar);

but with type Foo = impl Trait; trying to apply the same sort of rule you get to this recursive definition that needs a special case when you use a bare impl Trait in a type alias.

This comment has been minimized.

Copy link
@varkor

varkor Aug 5, 2018

Author Member

Ah, I see. Yes, you do have to have a single type alias as a base case if you're using the impl Trait type aliases to desugar. In practice, the desugaring of existential type is effectively replaced by the type alias. That is, the existential type design was originally intended to act as a desugaring for return-position and variable-binding (e.g. let) impl Trait. Using type aliases fills that role: but you can't use it to desugar itself. In practice, I don't think this is important, as there's no practical difference between the existential type itself and its alias.

This comment has been minimized.

Copy link
@alexreg

alexreg Aug 6, 2018

Why can't it be used to desugar, @varkor?

This comment has been minimized.

Copy link
@varkor

varkor Aug 6, 2018

Author Member

Because if you try to desugar each occurrence of impl Trait you would end up trying to desugar:

type Foo = impl Trait;

into:

type Foo = impl Trait;

so you need to have this as a base case.

@Centril
Copy link
Contributor

left a comment

Some nits

Show resolved Hide resolved text/0000-impl-trait-type-aliases.md Outdated
Show resolved Hide resolved text/0000-impl-trait-type-aliases.md Outdated
Show resolved Hide resolved text/0000-impl-trait-type-aliases.md Outdated
Show resolved Hide resolved text/0000-impl-trait-type-aliases.md Outdated
Show resolved Hide resolved text/0000-impl-trait-type-aliases.md Outdated
Show resolved Hide resolved text/0000-impl-trait-type-aliases.md Outdated
@clarfon

This comment has been minimized.

Copy link
Contributor

commented Aug 5, 2018

Just to clarify because I'm not 100% certain on how impl Trait works, these will allow where clauses too, right? In other words, type Thing = impl Trait where Self: Clone.

If I recall correctly, impl Trait doesn't allow this, as it'd be ambiguous if you said fn fun() -> impl Trait where Self: Clone because it's unclear whether the Self: Clone applies to the impl Trait or the fn fun().

So technically where clauses would have to be added to make this equivalent to the existential type syntax. It also solves the problem of where clauses for impl Trait, as you can make a type alias if you need them.

I think the RFC should have some discussion of where clauses. I didn't see any.

@Nemo157

This comment has been minimized.

Copy link
Contributor

commented Aug 5, 2018

@clarcharr what's the intended meaning of where clauses on an impl Trait? I can't work out what type Thing = impl Trait where Self: Clone; would do, or why you would use it.

@clarfon

This comment has been minimized.

Copy link
Contributor

commented Aug 5, 2018

So I just used Self: Clone as an example but a better one might be, for example, impl Iterator where Self::Item: Clone. While in the future this will be doable as impl Iterator<Item: Clone> you need a where clause for this currently.

In general, there are going to be things you can't express without where clauses, and it'd be nice to allow this sort of thing.

@Nemo157

This comment has been minimized.

Copy link
Contributor

commented Aug 5, 2018

Thanks, that clarifies things a lot for me (I think putting a bound directly on Self really threw me since that can be just a + Bound on the trait). And I now remember I actually raised similar points on the RFC that introduced impl Iterator<Item: Clone> (#2289 in case anyone is interested).

In the end I think I decided that that specific case acts identically to impl Iterator<Item = impl Clone>, which I think will cover 90%+ of use cases.

I'm trying to think of useful bounds that can't be written currently, I guess there could be something like impl Iterator where for<'a> &'a Self::Item: SomeTrait maybe?


Relatedly I was playing round with the current implementation and noticed that it allows specifying bounds in the type parameter list but not adding a where clause, that allowed me to come up with an example of a currently compiling -> impl Trait return type that's not representable without where clauses on the type alias (playground)

existential type Bar<T: IntoIterator>: Iterator<Item = <T::Item as IntoIterator>::Item>;

fn bar<T>(a: T) -> Bar<T>
where
    T: IntoIterator,
    T::Item: IntoIterator,
    <T::Item as IntoIterator>::Item: Clone,

so even if Self based where clauses is deemed out of scope since -> impl Trait doesn't require them I hope where clauses for constraining the generic parameters are available.

@clarfon

This comment has been minimized.

Copy link
Contributor

commented Aug 5, 2018

One thing that I also realised is that this could be accomplished by trait aliases:

trait Trait = Bound where for<'a> &'a Self: Bound
type Thing = impl Trait

So maybe where clauses aren't necessary and might in fact overcomplicate stuff. Still worth mentioning in the RFC, though, as this is something the existential type syntax supported that the new syntax won't.

@varkor

This comment has been minimized.

Copy link
Member Author

commented Aug 5, 2018

Sorry, I'll get the bounds question soon, but in the meantime, I've noticed that this RFC has a major incompatible flaw with the original RFC 2071 and it could potentially mean this syntax suggestion is misinformed.

The current implementation of existential type, which I was using for reference, makes the underlying (inferred) type hidden to the enclosing module. This is consistent with impl Trait and is a motivating factor for the syntax proposed here. However, RFC 2071 explicitly states that the underlying type is transparent within the enclosing module.

As such, existential type and impl Trait are inconsistent with one another. Either:
(a) the feature as implemented right now must be accepted as having the correct semantics
(b) existential type cannot use the impl Trait syntax.

Note that, contrary to some of the original stated motivations, this feature means that existential type cannot be used as a desugaring for impl Trait.

@KodrAus

This comment has been minimized.

Copy link
Contributor

commented Aug 5, 2018

However, RFC 2071 explicitly states that the underlying type is visible within the enclosing module.

I did a quick mobile grok of #2071. Is it the reference section you're talking about where the existential type can be used as its resolved type within its declaring module? What's the tradeoff we're making by excluding this transparency from our model here?

@KodrAus

This comment has been minimized.

Copy link
Contributor

commented Aug 5, 2018

However, RFC 2071 explicitly states that the underlying type is visible within the enclosing module.

That doesn't seem inconsistent to my mental model actually. I've been thinking of impl Trait in terms of who decides what Trait is?. For return position, the function body decides what Trait is, and that concrete type is visible to the function body. For argument position, the caller decides what Trait is, and that concrete type is visible to the caller. For module position, the module decides what Trait is, and that concrete type is visible to the module.

@varkor

This comment has been minimized.

Copy link
Member Author

commented Aug 5, 2018

Is it the reference section you're talking about where the existential type can be used as its resolved type within its declaring module? What's the tradeoff we're making by excluding this transparency from our model here?

That's right. If we include the transparency, it's inconsistent with impl Trait, which is confusing as now impl Trait may or may not be transparent depending on the location. If we exclude transparency, then it's a change of behaviour from RFC 2071. This might be solvable, but requires some care.

That doesn't seem inconsistent to my mental model actually. I've been thinking of impl Trait in terms of who decides what Trait is?.

Yes, it's quite subtle. It could be consistent with argument-position, return-position and module-position impl Trait. However, it's not consistent with impl Trait in let bindings (another consequence of RFC 2071 that's not yet implemented). The simplest solution is probably altering impl Trait's behaviour in these circumstances to make it transparent.

@KodrAus

This comment has been minimized.

Copy link
Contributor

commented Aug 5, 2018

However, it's not consistent with impl Trait in let bindings

Ah right, I'd totally forgotten about let bindings! In that case, I'd expect the expression part of the binding to decide what Trait is, and that concrete type to be visible only within that expression. For example:

let mut x: impl Debug = {
    if a { 1 }
    else { some_fn_returning_i32() }
};

x = 1; // err: expected `impl Debug` got `i32`
@Nemo157

This comment has been minimized.

Copy link
Contributor

commented Feb 26, 2019

@alexreg that’s not a desugaring, that’s a manual splitting of the type alias, maybe because the author has decided that being able to name the inner type individually would be useful. This would work under the “always existential” rule, but wouldn’t with the type of the generic varying like you mention.

@alexreg

This comment has been minimized.

Copy link

commented Feb 26, 2019

@Nemo157 Oh, I see what you mean. Yes... we need to think about how best to teach that, since that intuitive splitting does of course change semantics.

@rpjohnst

This comment has been minimized.

Copy link

commented Feb 26, 2019

@dhardy

Centril made a very nice argument about why this may be the best candidate syntax for existential types, and now we have confusion over whether the syntax is used for existentials (as in RPIT) or sugar for a hidden type parameter (as in APIT).

This is a misunderstanding of the term "existential" and the whole reason we are moving away from that description.

APIT and hidden type parameters are existential- the types ∀a.fn(a) and fn(∃a.a) are isomorphic. Both accept any value the caller decides to throw at them. It is RPIT that is not really an existential, because the callee cannot produce any value it likes, but is limited to a single actual type.

The proposed interpretation is instead "impl means infer an opaque type." APIT and RPIT are still a bit different here, but much closer- their behavior is analogous to ML-like type inference for functions. And if you look at the Fn traits, you can see a concrete reason for the remaining conceptual difference: function parameters correspond to trait parameters, while the return type corresponds to an associated type.

@dhardy

This comment has been minimized.

Copy link
Contributor

commented Feb 26, 2019

@rpjohnst I agree that "existential" is not really the correct terminology (if you look up 20 or so posts, you'll see I suggested some alternatives). It does appear to be what everyone is using, but you are right: calling these "opaque types" is more accurate.

There is however a very important difference. E.g. is the following legal? Is S well defined as a type, or is it generic? Is the current scope parametrised with a hidden type such that S is only well defined locally?

type T = impl Trait;
struct S {
    value: T
}
// ... some more code to constrain the type of T sufficiently

What I would personally like to see is S being a well-defined type globally; even one that can be exported as part of the public API.

@alexreg

This comment has been minimized.

Copy link

commented Feb 26, 2019

@dhardy I would expect that in your example S would not be implicitly parametrised, but rather T would be an inferred opaque type, but concrete. We may want different semantics for something like struct S { value: impl Trait } though, where S would actually be parametrised. (Probably not, but I just thought I'd leave it open, since @Centril may have ideas about this.)

@Nemo157

This comment has been minimized.

Copy link
Contributor

commented Feb 26, 2019

I just realised the issue I have with @alexreg's desugaring above that was hiding the use of APIT. Specifically, concrete types should be transparent to the APIT/RPIT distinction (as evidenced by my playground above):

type Foo = Arc<impl Trait>;

should be the equivalent of what is written today as

type Foo = Arc<_0>;
existential type _0: Trait;

This follows from treating type Foo as the same base context as RPIT, you can have -> Arc<impl Trait> which means that the function chooses the type inside the Arc.


The interesting question is what should happen when you have nested impl Trait inside impl Trait, something like

type Bar = impl std::ops::Add<impl Into<i32>, Output = impl std::fmt::Display>;

fn bar() -> Bar { ... }

this could take the form that @alexreg expands above where input type parameters to traits get flipped, but that seems to require introducing a non-local hidden generic type parameter onto functions (it would even be possible to introduce this hidden parameter onto a function outside the current module if it returns a value it got from calling a function inside the module)

type Bar<_0: Into<i32>> = _1<_0>;
existential type _1<_2: Into<i32>>: std::ops::Add<_2, Output = _3>;
existential type _3: std::fmt::Display;

fn bar<_4: Into<i32>>() -> Bar<_4> { ... }
EDIT: Unless higher ranked type-generic bounds were a thing, maybe?
type Bar = _1;
existential type _1: for<_2: Into<i32>> std::ops::Add<_2, Output = _3>;
existential type _3: std::fmt::Display;

fn bar() -> Bar { ... }

Looking at what happens if you try and write the equivalent today, as an RPIT:

fn bar() -> impl std::ops::Add<impl Into<i32>, Output = impl std::fmt::Display> { ... }

gives `impl Trait` not allowed outside of function and inherent method return types on the impl Into<i32> (but is ok with the impl std::fmt::Display as the output, so that message is technically wrong), while

existential type Bar: std::ops::Add<impl Into<i32>, Output = impl std::fmt::Display>;

gives the same error on both uses of impl Trait.

@rpjohnst

This comment has been minimized.

Copy link

commented Feb 26, 2019

@dhardy

What I would personally like to see is S being a well-defined type globally; even one that can be exported as part of the public API.

This matches the proposal. That use of impl Trait is not in a function argument, so it doesn't correspond to a trait parameter from the Fn traits, or a location where ML-like inference would find polymorphism, so it does exactly what you want.

@Centril

This comment has been minimized.

Copy link
Contributor

commented Feb 26, 2019

@alexreg

The third case also has a relationship to associated type bounds. For example, trait Qux = Iterator<Item: impl Trait>; would desugar to trait Qux = Iterator<Item = impl Trait>;, which in turn would desugar as per the above example. (Note that the case of associated type bounds within trait aliases isn't explicitly covered by that RFC, but there's no reason we shouldn't support it too.)

If we map out where the RFC would naturally lead, it would desugar to trait Qux = Iterator where <Self as Iterator>::Item: Trait;. The RFC specifies it in terms of where-clauses; that this has equal semantics to impl Trait in some circumstances is incidental.


@Nemo157 Your desugaring is wrong, I believe.

type Foo = Arc<impl Iterator<Item = impl Debug>>;

-->

type Foo<T: Iterator<Item = _0>> = Arc<T>;
existential type _0: Debug;

That's certainly what I would expect.

Under this RFC, type Foo = Arc<impl Iterator<Item = impl Debug>>; should be equivalent to:

existential type _0: Debug;
existential type _1: Iterator<Item = _0>;
type Foo = Arc<_1>;

@varkor, can you please clarify this in the reference (and in general how nested impl traits work where they are allowed (but notably not impl Foo<impl Bar> since it isn't allowed anywhere today...))?


@dhardy

There is however a very important difference. E.g. is the following legal? Is S well defined as a type, or is it generic? Is the current scope parametrised with a hidden type such that S is only well defined locally?

type T = impl Trait;
struct S {
    value: T
}
// ... some more code to constrain the type of T sufficiently

From the perspective of type inference with opacity, let's desugar your example (with defining use fn foo):

type T = forget(?0, Trait);
struct S { value: T }

pub fn foo() -> T { 42u8 }

Here, ?0 denotes a unification variable. Meanwhile, forget(X, Y) takes a type X and forgets the type identity ("details" re. scoping omitted...) and leaves Y as the interface you can assume about X. Specifically, we take ?0 and make it opaque such that only Trait can be assumed to hold about ?0.

Type aliases are aliases and therefore every monomorphic use of them should refer to the same type. Combined with ?0 being an inference variable, it is quite natural that we equate ?0 to a specific type (given by foo, so u8) and then forget exactly what type it was.


@alexreg

We may want different semantics for something like struct S { value: impl Trait } though, where S would actually be parametrised. (Probably not, but I just thought I'd leave it open, since @Centril may have ideas about this.)

Probably not :) If we have value: impl Trait, I would assign the existential type meaning to the type of value.


@Nemo157

The interesting question is what should happen when you have nested impl Trait inside impl Trait, something like

type Bar = impl std::ops::Add<impl Into<i32>, Output = impl std::fmt::Display>;

fn bar() -> Bar { ... }

This is an error for the same reason that -> impl Foo<impl Bar> is an error today. See rust-lang/rust#57979 (comment). In particular, it could mean: -> impl for<T: Bar> Foo<T>.

@pnkfelix

This comment has been minimized.

Copy link
Member

commented Mar 13, 2019

@Centril wrote

To unlock the composability benefits of this RFC and avoid needless surprises, I also propose that we immediately resolve the remaining unresolved question in the RFC in favour of lifting the restriction. (@varkor, please edit the RFC accordingly once we're in FCP).

One great property of this RFC as written is that it is truly just resolving a syntactic question, without attempting to add any new expressiveness on top of the current implementation.

Correct me if I misunderstand what @Centril wrote, but it seems like the call for a final comment period is suggesting an implicit amendment to the RFC that breaks that property.

  • In other words, I don't know if the FCP summary comment is trying to say that we are checking off approval on a document that has this has-yet-unwritten change in place.

I understand the goal of trying to strive to maximize uniformity and expressive power.

But I worry about an overall issue in the RFC process, where an implicit change is proposed to the RFC via a single sentence in 2,000 word summary comment (a comment that is otherwise just condensing the dialogue up to this point, and not attempting to introduce new ideas).

  • Its a bad precedent to set
  • It increases the chance of team members having different mental models of what change is being proposed.

Having said that, I don't care enough about the potential for process-breakdown to actually block the FCP on this procedural matter. I just wanted to voice my opinion on the matter.

@pnkfelix

This comment has been minimized.

Copy link
Member

commented Mar 13, 2019

@rfcbot reviewed

@varkor

This comment has been minimized.

Copy link
Member Author

commented Mar 13, 2019

One great property of this RFC as written is that it is truly just resolving a syntactic question, without attempting to add any new expressiveness on top of the current implementation.

To add to this point, I think there is no disadvantage to leaving the point in question as an open question. One straightforward possibility is to add an additional feature gate that permits this extended use of impl Trait to allow experimentation without extending the scope of this RFC past a syntactic change.

@Centril

This comment has been minimized.

Copy link
Contributor

commented Mar 13, 2019

Correct me if I misunderstand what @Centril wrote, but it seems like the call for a final comment period is suggesting an implicit amendment to the RFC that breaks that property.

It suggests resolving the existing unresolved question. To make this clearer and to ensure that the document is revised, I'll add concerns to that end:

@rfcbot concern resolve-unresolved-question-in-favor-of-allowing-it
@rfcbot concern clarify-that-multiple-impl-traits-have-usual-current-semantics

One great property of this RFC as written is that it is truly just resolving a syntactic question, without attempting to add any new expressiveness on top of the current implementation.

I think this remains the case. E.g. consider:

#![feature(existential_type)]

existential type _0: core::fmt::Debug;
existential type _1: core::fmt::Debug;
type _2 = (_0, _1);
fn foo() -> _2 { (1, 2) }

existential type _3: core::fmt::Debug;
existential type _4: Iterator<Item = _3>;
fn bar() -> _4 { 0..1 }

fn main() {}

This compiles fine on nightly and would be rewritten as:

type _2 = (impl core::fmt::Debug, impl core::fmt::Debug);
fn foo() -> _2 { (1, 2) }

type _4 = impl Iterator<Item = impl core::fmt::Debug>;
fn bar() -> _4 { 0..1 }
@vi

This comment has been minimized.

Copy link

commented Mar 26, 2019

Should there be a rule that impl in pub type Smth should be solitary and unwrapped?

Consider two APIs:

pub type Thing = impl Traitor;
pub type ThingR = Result<Thing, ()>;
pub type ThingR = Result<impl Traitor, ()>;

First one is OK. But in the second one it is less obvious how to make Option<Thing>, especially without using another type ... = impl .... There is obvious conversion from Result to Option, but I suspect it can also be tricky in general case.

Is using non-straightforward definition of existential public type a case of private-in-public?

@Centril

This comment has been minimized.

Copy link
Contributor

commented Mar 27, 2019

Should there be a rule that impl in pub type Smth should be solitary and unwrapped?

Just as you may write:

fn foo() -> Option<impl Iterator<Item = impl Debug>> { Some(0..1) }

so too should you be able to write:

type FoosType = Option<impl Iterator<Item = impl Debug>>;
fn foo() -> FoosType { Some(0..1) }

I think there's little justification to intentionally make the language less composable in this way. It doesn't make the language simpler. Rather, a surprising ad-hoc rule is imposed that people will need to learn.

First one is OK. But in the second one it is less obvious how to make Option<Thing>, especially without using another type ... = impl .... There is obvious conversion from Result to Option, but I suspect it can also be tricky in general case.

It should not be any less obvious than type Foo = Option<u8>; if you understand how type A = impl B and generic parameters/arguments work. If you have type Foo = Option<impl Bar>;, in a function which defines Foo, simply provide a None or Some(x) where x: impl Bar holds. The same applies to x: u8.

Is using non-straightforward definition of existential public type a case of private-in-public?

No more than pub Foo = Box<dyn Trait>; is. It is not a case of private-in-public because the act of using impl Trait obscures the underlying type identity. The RFC gives a good justification when it talks about __foo_return. Indeed we may already write fn x() -> impl Clone { #[derive(Clone)] struct A; A } which smuggles a private type A out of the function.

@Centril Centril added the I-nominated label Mar 27, 2019

@joshtriplett

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

The RFC should explicitly state (in the summary and in the motivation) that this covers impl Trait in both type aliases and in associated types.

@varkor

This comment has been minimized.

Copy link
Member Author

commented Mar 28, 2019

I've clarified the semantics for multiple impl Traits in a single alias, as well as stated that this syntax applies to both type aliases and associated types.

Edit: after discussion with @Centril, I've resolved the final question in the RFC, permitting multiple occurrences of impl Trait.

@Centril

This comment has been minimized.

Copy link
Contributor

commented Mar 28, 2019

Thanks!

@rfcbot resolve resolve-unresolved-question-in-favor-of-allowing-it
@rfcbot resolve clarify-that-multiple-impl-traits-have-usual-current-semantics

@Centril Centril removed the I-nominated label Apr 18, 2019

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Apr 18, 2019

We discussed this in the @rust-lang/lang meeting. @withoutboats said that they plan to spend some time thinking about this and to write up their thoughts within the next 2 weeks.

@varkor

This comment has been minimized.

Copy link
Member Author

commented May 2, 2019

@rust-lang/lang: if this still won't have been resolved before today's meeting, could it be added to the agenda?

@Centril Centril added the I-nominated label May 2, 2019

@scottmcm

This comment has been minimized.

Copy link
Member

commented May 9, 2019

I don't think I'm convinced, but I don't feel strongly enough to block. Consider me abstaining here.

@rfcbot reviewed

@cramertj

This comment has been minimized.

Copy link
Member

commented May 9, 2019

Ping @withoutboats this is still blocked on your feedback.

@cramertj

This comment has been minimized.

Copy link
Member

commented May 14, 2019

Pinging @withoutboats again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.