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

Tracking issue for RFC 1951: Finalize syntax and parameter scoping for `impl Trait`, while expanding it to arguments #42183

Open
aturon opened this Issue May 24, 2017 · 13 comments

Comments

Projects
None yet
@aturon
Member

aturon commented May 24, 2017

This is a tracking issue for the RFC "Finalize syntax and parameter scoping for impl Trait, while expanding it to arguments" (rust-lang/rfcs#1951).

Steps:

Unresolved questions:

  • The precedence rules around impl Trait + 'a need to be nailed down.

Known bugs:

(this list is not necessarily exhaustive)

@droundy

This comment has been minimized.

Show comment
Hide comment
@droundy

droundy Jul 6, 2017

Contributor

I would hate to slow up impl Trait (which I am very eager for), but I am disappointed that it does not support generative existentials. Specifically, I could imagine a scenario such as

enum Foo {
   Vec(Vec<u8>),
   Set(HashSet<u8>,
}

fn mkiter(f: Foo) -> impl Iterator<Item=&u8> {
    match f {
      Foo::Vec(ref v) => v.iter(),
      Foo::Set(ref s) => s.iter(),
    }
}

Currently I could do this with a trait object, but that would expose the dynamicness in my API. So instead I would create a new enum to hold the iterator. It looks like with impl Trait as proposed, I still have to create my new enum and implement Iterator on that enum, and then I can return an impl Trait?

Contributor

droundy commented Jul 6, 2017

I would hate to slow up impl Trait (which I am very eager for), but I am disappointed that it does not support generative existentials. Specifically, I could imagine a scenario such as

enum Foo {
   Vec(Vec<u8>),
   Set(HashSet<u8>,
}

fn mkiter(f: Foo) -> impl Iterator<Item=&u8> {
    match f {
      Foo::Vec(ref v) => v.iter(),
      Foo::Set(ref s) => s.iter(),
    }
}

Currently I could do this with a trait object, but that would expose the dynamicness in my API. So instead I would create a new enum to hold the iterator. It looks like with impl Trait as proposed, I still have to create my new enum and implement Iterator on that enum, and then I can return an impl Trait?

@eddyb

This comment has been minimized.

Show comment
Hide comment
@eddyb

eddyb Jul 6, 2017

Member

@droundy That came up a few times, as creating an "intersection type", in your case it would be something like vec::Iter<u8> | hash_set::Iter<u8>, which would dispatch its methods to whichever type it was constructed from. The big difference from the other proposal with that syntax, "anonymous sum types", is that an intersection type can't be matched, as T | T collapses to T, but as a sum type, you have to distinguish between the two T "variants", so either match doesn't work in some cases orT | T is banned somehow (almost impossible with generics).

I am actually in favor of doing it, for impl Trait or even in more cases, when multiple choices of type exist (if-else, match arms, array elements etc.) and they conflict between themselves.
That is, only code that currently doesn't compile would automatically get T | U types.

Member

eddyb commented Jul 6, 2017

@droundy That came up a few times, as creating an "intersection type", in your case it would be something like vec::Iter<u8> | hash_set::Iter<u8>, which would dispatch its methods to whichever type it was constructed from. The big difference from the other proposal with that syntax, "anonymous sum types", is that an intersection type can't be matched, as T | T collapses to T, but as a sum type, you have to distinguish between the two T "variants", so either match doesn't work in some cases orT | T is banned somehow (almost impossible with generics).

I am actually in favor of doing it, for impl Trait or even in more cases, when multiple choices of type exist (if-else, match arms, array elements etc.) and they conflict between themselves.
That is, only code that currently doesn't compile would automatically get T | U types.

@droundy

This comment has been minimized.

Show comment
Hide comment
@droundy

droundy Jul 7, 2017

Contributor

If I'm understanding you, @eddyb, this means that this is case won't work in the current proposal, but there is room for a backwards compatible change that will enable automatic generation of sum types in the future?

Contributor

droundy commented Jul 7, 2017

If I'm understanding you, @eddyb, this means that this is case won't work in the current proposal, but there is room for a backwards compatible change that will enable automatic generation of sum types in the future?

@eddyb

This comment has been minimized.

Show comment
Hide comment
@eddyb

eddyb Jul 7, 2017

Member

Yes, it only showed up in related discussion but is not part of any accepted proposal.

Member

eddyb commented Jul 7, 2017

Yes, it only showed up in related discussion but is not part of any accepted proposal.

@cramertj

This comment has been minimized.

Show comment
Hide comment
@cramertj

cramertj Jul 21, 2017

Member

Before landing anything here we should make sure we correctly handle (or error on) lifetime elision in impl Trait. See #43396.

The more conservative option is to just ban elision entirely for the time being.

Member

cramertj commented Jul 21, 2017

Before landing anything here we should make sure we correctly handle (or error on) lifetime elision in impl Trait. See #43396.

The more conservative option is to just ban elision entirely for the time being.

@skade

This comment has been minimized.

Show comment
Hide comment
@skade

skade Aug 28, 2017

Contributor

The case @droundy describes has a pretty common equivalent in futures, basically anywhere the Either future is currently used. I often reach for it when futures code branches on a condition (for example, something existing already or having to be created first.

Contributor

skade commented Aug 28, 2017

The case @droundy describes has a pretty common equivalent in futures, basically anywhere the Either future is currently used. I often reach for it when futures code branches on a condition (for example, something existing already or having to be created first.

@nikomatsakis

This comment has been minimized.

Show comment
Hide comment
@nikomatsakis

nikomatsakis Aug 28, 2017

Contributor

I've certainly encountered the desire for | types in practice. I am a bit wary of them (although it occurs to me that they could be a helpful way to avoid the need to compute LUB for thorny cases), but it'd be a nice way to resolve this problem. I would certainly like for most users to not be forced to be aware of their existence. At least trait authors would presumably have to be, though, since you would probably have to provide something like impl<A,B> Future for (A|B). We could maybe synthesize such impls, but not, I think, in all cases (the restrictions are probably similar-ish to object safety).

Contributor

nikomatsakis commented Aug 28, 2017

I've certainly encountered the desire for | types in practice. I am a bit wary of them (although it occurs to me that they could be a helpful way to avoid the need to compute LUB for thorny cases), but it'd be a nice way to resolve this problem. I would certainly like for most users to not be forced to be aware of their existence. At least trait authors would presumably have to be, though, since you would probably have to provide something like impl<A,B> Future for (A|B). We could maybe synthesize such impls, but not, I think, in all cases (the restrictions are probably similar-ish to object safety).

@droundy

This comment has been minimized.

Show comment
Hide comment
@droundy

droundy Aug 28, 2017

Contributor

@nikomatsakis I think there is a big difference between type-visible intersection types and those that would/could be hidden behind impl Trait. The latter would not require any users to be aware of their existence, and thus (I think) would not introduce this sort of complexity. It would just lift the restriction that a function returning impl Trait must return the same concrete type on each branch, at the cost of runtime checks generated by the compiler to determine which concrete type actually was returned.

Contributor

droundy commented Aug 28, 2017

@nikomatsakis I think there is a big difference between type-visible intersection types and those that would/could be hidden behind impl Trait. The latter would not require any users to be aware of their existence, and thus (I think) would not introduce this sort of complexity. It would just lift the restriction that a function returning impl Trait must return the same concrete type on each branch, at the cost of runtime checks generated by the compiler to determine which concrete type actually was returned.

@eddyb

This comment has been minimized.

Show comment
Hide comment
@eddyb

eddyb Aug 28, 2017

Member

We could maybe synthesize such impls, but not, I think, in all cases (the restrictions are probably similar-ish to object safety).

If you allow user matching on such types which is needed for manual impls, then T | T becomes a problem - IMO synthesized impls should be the only thing you can do with these types.

Member

eddyb commented Aug 28, 2017

We could maybe synthesize such impls, but not, I think, in all cases (the restrictions are probably similar-ish to object safety).

If you allow user matching on such types which is needed for manual impls, then T | T becomes a problem - IMO synthesized impls should be the only thing you can do with these types.

@plietar

This comment has been minimized.

Show comment
Hide comment
@plietar

plietar Aug 29, 2017

Contributor

I wrote a little bit about this a while ago, including what implementations could be synthesized automatically, and how trait authors could specify their own :
https://internals.rust-lang.org/t/pre-rfc-anonymous-enum/4806

Contributor

plietar commented Aug 29, 2017

I wrote a little bit about this a while ago, including what implementations could be synthesized automatically, and how trait authors could specify their own :
https://internals.rust-lang.org/t/pre-rfc-anonymous-enum/4806

@nikomatsakis

This comment has been minimized.

Show comment
Hide comment
@nikomatsakis

nikomatsakis Aug 29, 2017

Contributor

@droundy

My point is merely that we cannot always synthesize impls. I might be fine with saying "when the traits meets the required criteria, we will permit | types in impl trait", but then this becomes a "silent propery" of the trait that one must be careful not to violate. We have some precedent fro this (object safety), but it's not one of the most loved features of the language (for many reasons...). I've not read @plietar's post yet (or at least not in a while...), but I guess that they enumerated some examples, so I won't bother to do so here. =)

Contributor

nikomatsakis commented Aug 29, 2017

@droundy

My point is merely that we cannot always synthesize impls. I might be fine with saying "when the traits meets the required criteria, we will permit | types in impl trait", but then this becomes a "silent propery" of the trait that one must be careful not to violate. We have some precedent fro this (object safety), but it's not one of the most loved features of the language (for many reasons...). I've not read @plietar's post yet (or at least not in a while...), but I guess that they enumerated some examples, so I won't bother to do so here. =)

@scottmcm

This comment has been minimized.

Show comment
Hide comment
@scottmcm

scottmcm Sep 18, 2017

Member

A thought from dyn Trait discussions: what about blanket impls for other traits?

With this + dyn, we have

fn foo(x: &dyn Trait)
fn foo(x: impl Trait)

impl Trait1 for dyn Trait2 { ... }
impl<T:Trait2> Trait1 for T { ... }

Should the last of those also be "argument position" and thus this? (Eventually, at least.)

impl Trait1 for impl Trait2 { ... }

That's sortof logical, and Self can be used to refer to the actual type. But it's also kinda weird to look at. Not that I really want to re-open the bikeshed...

Member

scottmcm commented Sep 18, 2017

A thought from dyn Trait discussions: what about blanket impls for other traits?

With this + dyn, we have

fn foo(x: &dyn Trait)
fn foo(x: impl Trait)

impl Trait1 for dyn Trait2 { ... }
impl<T:Trait2> Trait1 for T { ... }

Should the last of those also be "argument position" and thus this? (Eventually, at least.)

impl Trait1 for impl Trait2 { ... }

That's sortof logical, and Self can be used to refer to the actual type. But it's also kinda weird to look at. Not that I really want to re-open the bikeshed...

bors added a commit that referenced this issue Nov 17, 2017

Auto merge of #45701 - cramertj:impl-trait-this-time, r=eddyb
impl Trait Lifetime Handling

This PR implements the updated strategy for handling `impl Trait` lifetimes, as described in [RFC 1951](https://github.com/rust-lang/rfcs/blob/master/text/1951-expand-impl-trait.md) (cc #42183).

With this PR, the `impl Trait` desugaring works as follows:
```rust
fn foo<T, 'a, 'b, 'c>(...) -> impl Foo<'a, 'b> { ... }
// desugars to
exists type MyFoo<ParentT, 'parent_a, 'parent_b, 'parent_c, 'a, 'b>: Foo<'a, 'b>;
fn foo<T, 'a, 'b, 'c>(...) -> MyFoo<T, 'static, 'static, 'static, 'a, 'b> { ... }
```
All of the in-scope (parent) generics are listed as parent generics of the anonymous type, with parent regions being replaced by `'static`. Parent regions referenced in the `impl Trait` return type are duplicated into the anonymous type's generics and mapped appropriately.

One case came up that wasn't specified in the RFC: it's possible to write a return type that contains multiple regions, neither of which outlives the other. In that case, it's not clear what the required lifetime of the output type should be, so we generate an error.

There's one remaining FIXME in one of the tests: `-> impl Foo<'a, 'b> + 'c` should be able to outlive both `'a` and `'b`, but not `'c`. Currently, it can't outlive any of them. @nikomatsakis and I have discussed this, and there are some complex interactions here if we ever allow `impl<'a, 'b> SomeTrait for AnonType<'a, 'b> { ... }`, so the plan is to hold off on this until we've got a better idea of what the interactions are here.

cc #34511.
Fixes #44727.

bors added a commit that referenced this issue Nov 18, 2017

Auto merge of #45701 - cramertj:impl-trait-this-time, r=eddyb
impl Trait Lifetime Handling

This PR implements the updated strategy for handling `impl Trait` lifetimes, as described in [RFC 1951](https://github.com/rust-lang/rfcs/blob/master/text/1951-expand-impl-trait.md) (cc #42183).

With this PR, the `impl Trait` desugaring works as follows:
```rust
fn foo<T, 'a, 'b, 'c>(...) -> impl Foo<'a, 'b> { ... }
// desugars to
exists type MyFoo<ParentT, 'parent_a, 'parent_b, 'parent_c, 'a, 'b>: Foo<'a, 'b>;
fn foo<T, 'a, 'b, 'c>(...) -> MyFoo<T, 'static, 'static, 'static, 'a, 'b> { ... }
```
All of the in-scope (parent) generics are listed as parent generics of the anonymous type, with parent regions being replaced by `'static`. Parent regions referenced in the `impl Trait` return type are duplicated into the anonymous type's generics and mapped appropriately.

One case came up that wasn't specified in the RFC: it's possible to write a return type that contains multiple regions, neither of which outlives the other. In that case, it's not clear what the required lifetime of the output type should be, so we generate an error.

There's one remaining FIXME in one of the tests: `-> impl Foo<'a, 'b> + 'c` should be able to outlive both `'a` and `'b`, but not `'c`. Currently, it can't outlive any of them. @nikomatsakis and I have discussed this, and there are some complex interactions here if we ever allow `impl<'a, 'b> SomeTrait for AnonType<'a, 'b> { ... }`, so the plan is to hold off on this until we've got a better idea of what the interactions are here.

cc #34511.
Fixes #44727.

bors added a commit that referenced this issue Nov 21, 2017

Auto merge of #45701 - cramertj:impl-trait-this-time, r=eddyb
impl Trait Lifetime Handling

This PR implements the updated strategy for handling `impl Trait` lifetimes, as described in [RFC 1951](https://github.com/rust-lang/rfcs/blob/master/text/1951-expand-impl-trait.md) (cc #42183).

With this PR, the `impl Trait` desugaring works as follows:
```rust
fn foo<T, 'a, 'b, 'c>(...) -> impl Foo<'a, 'b> { ... }
// desugars to
exists type MyFoo<ParentT, 'parent_a, 'parent_b, 'parent_c, 'a, 'b>: Foo<'a, 'b>;
fn foo<T, 'a, 'b, 'c>(...) -> MyFoo<T, 'static, 'static, 'static, 'a, 'b> { ... }
```
All of the in-scope (parent) generics are listed as parent generics of the anonymous type, with parent regions being replaced by `'static`. Parent regions referenced in the `impl Trait` return type are duplicated into the anonymous type's generics and mapped appropriately.

One case came up that wasn't specified in the RFC: it's possible to write a return type that contains multiple regions, neither of which outlives the other. In that case, it's not clear what the required lifetime of the output type should be, so we generate an error.

There's one remaining FIXME in one of the tests: `-> impl Foo<'a, 'b> + 'c` should be able to outlive both `'a` and `'b`, but not `'c`. Currently, it can't outlive any of them. @nikomatsakis and I have discussed this, and there are some complex interactions here if we ever allow `impl<'a, 'b> SomeTrait for AnonType<'a, 'b> { ... }`, so the plan is to hold off on this until we've got a better idea of what the interactions are here.

cc #34511.
Fixes #44727.
@matthewjasper

This comment has been minimized.

Show comment
Hide comment
@matthewjasper

matthewjasper Jun 17, 2018

Contributor

Can this be closed? #34511 is the tracking issue for further development of impl trait/abstract type.

Contributor

matthewjasper commented Jun 17, 2018

Can this be closed? #34511 is the tracking issue for further development of impl trait/abstract type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment