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

Minimal `impl Trait` #1522

Merged
merged 19 commits into from Jun 27, 2016
Merged

Minimal `impl Trait` #1522

merged 19 commits into from Jun 27, 2016

Conversation

@Kimundi
Copy link
Member

Kimundi commented Mar 1, 2016

Add a initial, minimal form of impl Trait.

Rendered

[edited to link to final rendered version]

@eddyb
Copy link
Member

eddyb commented Mar 1, 2016

I should mention that being able to use @Trait within an associated type in a trait impl (which can be "fulfilled" by any of the methods in the same impl) provides maximal expressiveness:

  • a name for the anonymized type (type Anon = <T as Trait>::Assoc)
  • the ability to have extension methods that effectively return @Trait (and not just free functions)
  • regular old trait implementations that operate with types they can't name (e.g. an iterator that produces more iterators, containing closures)

That extension to this RFC was essential to my calendar demo implementation from last year, and had a small implementation footprint.

… as part of a return type of a freestanding function
@arielb1
Copy link
Contributor

arielb1 commented Mar 1, 2016

Type parameters and trait objects seem to work quite fine without OIBIT passthrough, so maybe this could work there too?

@steveklabnik
Copy link
Member

steveklabnik commented Mar 1, 2016

Bikeshed checking in: I strongly prefer a keyword like impl Trait over @Trait, as it's only barely longer, and is significantly more discoverable.

@ticki
Copy link
Contributor

ticki commented Mar 1, 2016

@steveklabnik I agree. @ seems like a very noisy sigil to reintroduce. However, impl isn't fit for this purpose, since this feature is like going to be used a lot, and abusing impl isn't really something we want to.

@eddyb
Copy link
Member

eddyb commented Mar 1, 2016

@steveklabnik Compare:

fn foo(x: bool) -> @Fn(&str) -> @Fn(i32) -> String {...}
fn foo(x: bool) -> impl Fn(&str) -> impl Fn(i32) -> String {...}
@steveklabnik
Copy link
Member

steveklabnik commented Mar 1, 2016

@ticki that might be fair, my preference is "keyword" over "sigil", the specific keyword is less important. I'm not totally sure how this is "absuing" impl, it's about symmetry.

@eddyb Sure. Prefer the second.

@steveklabnik
Copy link
Member

steveklabnik commented Mar 1, 2016

Anyway, I don't want to distract with the bikeshed here: Let's worry about semantics first. I will refrain from further expressing my preferences here until the end 😄

@comex
Copy link

comex commented Mar 2, 2016

+1 on impl over @.

Perhaps impl could be optional: writing impl Trait would always work, including in future extensions, but in unambiguous cases such as function return types, you could just write -> Trait. However, that might be confusing.

@aturon aturon self-assigned this Mar 2, 2016
@Kimundi
Copy link
Member Author

Kimundi commented Mar 2, 2016

@comex That would be a bit confusing though, and ambiguous with the hypothetical-but-talked-about extension to return DST values. (Which is theoretically possible given outpointers and DST-allocator/box support)

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Mar 2, 2016

I'm -1 on having no prefix at all; among other things, I'd eventually like to be able to use impl Trait in method argument position, and possibly other positions as well.

- The type would be known to implement the specified traits.
- The type would not be known to implement any other trait, with
the exception of OIBITS (aka "auto traits") and default traits like `Sized`.

This comment has been minimized.

Copy link
@petrochenkov

petrochenkov Mar 2, 2016

Contributor

Can the part about OIBITs be omitted at least initially, until the usage experience provides the evidence, that this is really really necessary and unavoidable?

This comment has been minimized.

Copy link
@nikomatsakis

nikomatsakis Mar 2, 2016

Contributor

On Wed, Mar 02, 2016 at 08:48:13AM -0800, Vadim Petrochenkov wrote:

Can the part about OIBITs be omitted at least initially, until the usage experience provide the evidence, that this is really really necessary and unavoidable?

It could, but it is clear that in the longer term we will want to
infer Send bounds. Certainly this is what makes generic combinator
libraries like iterators work smoothly with auto traits (OIBITs) like
send and sync. (Conversly, the fact that trait objects do NOT
permit this sort of "leakage" of auto trait information is a major
pain point with objects.)

To see why this would be limiting for generic combinator libaries,
imagine an example like this:

fn curry<F,U,V,R>(func: F, argument: U) -> @Fn(&V) -> R
    where F: Fn(&U, &V) -> R
{
    move |v| func(&argument, &v)
}

Now I have this nice little combinator that I can use to do stuff like
let foo = curry(Option::unwrap_or, some_opt). But now suppose that
I want to use Rayon to parallelize some computation:

fn my_computation() {
    let foo = curry(Option::unwrap_op, some_opt);
    rayon::join(
        || do_something(foo(22)),
        || do_something_else(foo(22)));
}

Now I get a compilation error because I am sharing foo, and all we
know about foo is that it implements Fn(&V) -> R; we don't
know whether it is Send.

Of course, I can modify curry to require that its inputs are send:

fn curry<F,U,V,R>(func: F, argument: U) -> @Fn(&V) -> R + Send
    where F: Fn(&U, &V) -> R + Send, U: Send
{
    move |v| func(&argument, &v)
}

but now I can't use curry for other cases, where the data is not
sendable. So I need to have curry_send and curry. etc.

That said, there is definitely a need to address this more generally.
For example, you will want to write iterator adapters that are
double-ended if the input is double-ended and so forth. We could
conceivably use the same mechanism to "propagate" Send as we invent
for handling DoubleEndedIterator. But on the other hand, the whole
point of traits like Send and Sync being auto traits is to ensure
that they are, well, automatic, and not something one has to opt in
to.

This comment has been minimized.

Copy link
@tikue

tikue Mar 3, 2016

What if there were a way to specify conditional OIBITs? Awful strawman (leaving the sigil, but I'm not necessarily advocating for it):

fn curry<F,U,V,R>(func: F, argument: U) -> @Fn(&V) -> R
    where F: Fn(&U, &V) -> R,
          U: Send + Sync for return
{
    move |v| func(&argument, &v)
}

This comment has been minimized.

Copy link
@eddyb

eddyb Mar 3, 2016

Member

What happens when an OIBIT is added, maybe even in a non-std crate?
The thing you're proposing has been considered and it might work if Send and Sync were the only OIBITs ever. But that's wrong, even right now.

This comment has been minimized.

Copy link
@nikomatsakis

nikomatsakis Mar 3, 2016

Contributor

I don't even know what an OIBIT is. Auto trait! Auto trait!

@thepowersgang
Copy link
Contributor

thepowersgang commented Sep 2, 2016

@norru - Tracking issue rust-lang/rust#34511

@steveklabnik
Copy link
Member

steveklabnik commented Sep 2, 2016

@norru also worth noting, last I heard, it was unclear if changing Iterator
to use this syntax is a breaking change or not. In the most straightforward
sense, it is, so that's some work that needs to be done... so even if it
does land, it might not fix that problem.

On Fri, Sep 2, 2016 at 7:24 AM, John Hodge (Mutabah) <
notifications@github.com> wrote:

@norru https://github.com/norru - Tracking issue rust-lang/rust#34511
rust-lang/rust#34511


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1522 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AABsis8Sc87QWHWM6DVIFSmaMl4T3P2Oks5qmAd2gaJpZM4HmxvM
.

@norru
Copy link

norru commented Sep 2, 2016

@steveklabnik It might cover my use case anyway. I'll give it a go as soon as it's available in the beta channel and then we'll see. Cheers!

@steveklabnik
Copy link
Member

steveklabnik commented Sep 2, 2016

Once it's in beta, it's already on its way to stable. If you really want to make sure it works for you, you should try it out on nightly, so that way you can point out problems before it's set to go stable.

@norru
Copy link

norru commented Sep 2, 2016

It makes sense - it's already in there, is it not? EDIT: just got nightly but no luck - I feel I'm missing something silly here.

nico@nico-700G7A:~/Projects/rust-oids$ rustup run nightly rustc -V
rustc 1.13.0-nightly (497d67d70 2016-09-01)
nico@nico-700G7A:~/Projects/rust-oids$ rustup run nightly rustc src/main.rs
error: expected type, found keyword `trait`
   --> src/backend/world/swarm.rs:100:31
    |
100 |   pub fn agents_iter(&self) -> trait Iterator<Item=&Agent> {
    |                                ^^^^^

error: aborting due to previous error
@eddyb
Copy link
Member

eddyb commented Sep 2, 2016

@norru The syntax is impl Trait - that is, impl is a keyword and Trait is any trait path.
Not sure where you got trait Trait from, I didn't see that in the RFC or comments (I hope there's no misleading information out there, this early, it would be a shame).

@norru
Copy link

norru commented Sep 4, 2016

@eddyb Ace, yes I got the syntax wrong for no reason except me being dense :).
Thanks.

@norru
Copy link

norru commented Sep 4, 2016

nico@nico-700G7A:~/Projects/rust-oids$ rustup run nightly cargo run --release
...
  Compiling rust-oids v0.2.0 (file:///home/nico/Projects/rust-oids)
...
rustc: /buildslave/rust-buildbot/slave/nightly-dist-rustc-linux/build/src/llvm/lib/IR/Instructions.cpp:263: void llvm::CallInst::init(llvm::FunctionType*, llvm::Value*, llvm::ArrayRef<llvm::Value*>, llvm::ArrayRef<llvm::OperandBundleDefT<llvm::Value*> >, const llvm::Twine&): Assertion `(i >= FTy->getNumParams() || FTy->getParamType(i) == Args[i]->getType()) && "Calling a function with a bad signature!"' failed.

How do I report it to the compiler team?

@eddyb
Copy link
Member

eddyb commented Sep 4, 2016

@norru You should open an issue on https://github.com/rust-lang/rust.

@gurry
Copy link

gurry commented Feb 15, 2017

May be a dumb question, but what if Rust allowed using a trait any place where you can use a concrete type today. In a way, it'll mean traits will (syntactically) behave akin to interfaces in mainstream languages. Not only will this resolve the questions posed here (i.e. whether to use impl or not) it will also make Rust easier to learn for new people.

What I mean is, if you can do this today:

struct MyType1;
struct MyType2;

fn myfunc(x: MyType1) -> MyType2 {}

it should also be possible to do this:

trait MyTrait1 {}
trait MyTrait2 {}

fn myfunc(x: MyTrait1) -> MyTrait2 {}

My feeling about the proposed new use of impl is that it is making the generics ever harder to grok for newbies by introducing one more concept to an already extravagant syntax (e.g. things like how lifetime annotations look similar to traits and you have two ways to specify trait constaints, inline and where clause). This will only add to the existing complexity.

@oli-obk
Copy link
Contributor

oli-obk commented Feb 15, 2017

@gurry: unfortunately there's an issue when you have the type Box<MyTrait>, because right now that is a fat pointer (pointer to value + pointer to vtable), but with the suggested change, this would be Box<TypeThatImplementsMyTrait>, and that's a breaking change.

These ambiguities were heavily discussed in #1603

@withoutboats
Copy link
Contributor

withoutboats commented Feb 15, 2017

@gurry Everyone would prefer if we could just say "you can use a trait any place where you can use a type," but the problem is what it means to do that. There are three obvious semantics (generics, dynamic dispatch, and the semantics in this proposal), all of which the language now supports with different syntaxes. None of them are redundant with the others.

@gnzlbg
Copy link
Contributor

gnzlbg commented Feb 15, 2017

@withoutboats I think that ideally, existential types and generics, which both use static dispatch, should have the same syntax, and trait objects, which uses dynamic dispatch, would have a different syntax.

Since that ship has sailed, I really like that this RFC introduces special syntax, impl Trait, for existential types. It does leave Rust in a bit of a weird spot where trait objects that use dynamic dispatch shares the syntax with generics which uses static dispatch, and the outlier is existential types which also uses static dispatch.

But this is just that: a weird spot in the design space. It could be improved by encouraging users to use a special syntax for dynamic dispatch in the future (in another RFC), which would leave Rust with 3 different syntaxes to denote different things (generics, existential, and dynamic dispatch).

Maybe that would not be that weird, but whether this is worth pursuing or not is kind of orthogonal to whether this RFC is stabilized as is. We have painted ourselves into a corner here, and as I see it, all the ways out that do not break the language involve some special syntax for impl Trait.

@withoutboats
Copy link
Contributor

withoutboats commented Feb 15, 2017

@gnzlbg at minimum there would need to be a distinct elaborated syntax for univerals/existentials; we can't tell which you want without examining the function body which would mean global type inference

@vi
Copy link

vi commented Feb 15, 2017

Was the fn generic(...) -> <Iterator> {...} syntax from the "Rust easier for beginners" thread already discussed here?

@gurry
Copy link

gurry commented Feb 16, 2017

Thanks guys. Just read the whole of #1603. I appreciate the whole picture a lot better now.

@bestouff
Copy link
Contributor

bestouff commented Jun 2, 2017

Will it be possible to use impl Trait with let ? Say:

let a impl MyTrait = value;

That would be way cleaner and readable IMHO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

You can’t perform that action at this time.