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

Introduce dyn keyword #1603

Closed
wants to merge 3 commits into from
Closed

Introduce dyn keyword #1603

wants to merge 3 commits into from

Conversation

ticki
Copy link
Contributor

@ticki ticki commented May 1, 2016

Introduce a keyword, dyn, for denoting dynamic dispatch, with the motivation
of avoid "accidental overhead" and making the costs more explicit, along with
deprecation of the old syntax.

Rendered.

@cramertj Edit: A variation on this RFC was merged in #2113.

@KalitaAlexey
Copy link

@ticki Rendered link is invalid.

@ticki
Copy link
Contributor Author

ticki commented May 1, 2016

Fixed.

@KalitaAlexey
Copy link

Is dyn keyword is reserved?

@ticki
Copy link
Contributor Author

ticki commented May 1, 2016

No, it's not. It need to be context dependent. I added a new commit explaining that.

@KalitaAlexey
Copy link

Why can't we make warning about dyn will become reserved keyword in two release loops?

@ticki
Copy link
Contributor Author

ticki commented May 1, 2016

Why should we? Adding it as context-dependent keyword won't cause breakage.

@KalitaAlexey
Copy link

@ticki Is it any harder to make it context-dependent?

@ticki
Copy link
Contributor Author

ticki commented May 1, 2016

Yes, but it is inevitable. At least two RFCs introducing context dependent keywords have been accepted. Adding a third one won't increase the complexity.

@ftxqxd
Copy link
Contributor

ftxqxd commented May 1, 2016

I’m theoretically in favour of this, but it seems kinda late to be doing this. I agree that the trait object syntax is too easy and implicit, and I’ve seen a fair few newcomers make the understandable mistake that trait objects are the way of writing functions that accept any value that implements a trait (rather than using generics, which is much more common), but this would cause an enormous amount of breakage (in Rust 2.0, when presumably the deprecation warning would be promoted to an error).

@ticki
Copy link
Contributor Author

ticki commented May 1, 2016

but this would cause an enormous amount of breakage

Deprecation doesn't require breakage.

@ticki
Copy link
Contributor Author

ticki commented May 1, 2016

I see way too many people doing e.g. Box<Fn(u32) -> u32>, where plain generics can do the job, and it is certainly not obvious that that requires dynamic dispatch, and even more so by &Fn(u32) -> u32, in which there is no hint about overhead.

I agree, that ideally this should have been done originally, but I don't think it is too late. I think that this is worth the turbulence, it will entail.

@ticki
Copy link
Contributor Author

ticki commented May 1, 2016

Although, ideally it shall be made into an error after several release cycle, but this RFC has no such requirement. It is also worth noting that the "private in public" RFC caused severe breakage, but the ecosystem seemed to handle it well, by simply keeping it a warning in several release cycles.

@KalitaAlexey
Copy link

As I said, I want to be that error in several release cycle.

@ticki
Copy link
Contributor Author

ticki commented May 1, 2016

I don't disagree with that, but as an initial point, this RFC suggest no such action. We can amend it later, if the consensus is positive to that.

@ticki
Copy link
Contributor Author

ticki commented May 1, 2016

@kennytm @jonas-schievink, you -1'd this. Could you elaborate on why? I guess it is the stability issues?

@petrochenkov
Copy link
Contributor

+ to @P1start
Before 1.0 I always secretly wanted traits and trait types to be syntactically different (a la Trait vs obj Trait), but now I'm not sure it's feasible.

@jonas-schievink
Copy link
Contributor

Yes, I'm not in favor of adding another way to express the same thing we already can, with no chance of removing the "old" way (since it's used so heavily). Pre-1.0, sure. I don't think the idea of making the dynamic dispatch cost explicit is bad in itself, but I don't think the current way is too implicit (for example, when seeing traits as generic parameters you know there's dynamic dispatch involved - or are there exceptions to this?).

@ticki
Copy link
Contributor Author

ticki commented May 1, 2016

I disagree. I see newcommers make the mistake all the time. There is a quite severe
overuse of trait objects in the Rust community, particularly by new beginners.

It is certainly not obvious that wrapping a trait in some pointer will allow values tos
coerce into a trait object. In fact, it is quite counterintuitive.

That a trait (which isn't even a type) can coerce into a trait object (an usized type) can
be quite hard to understand.

@jonas-schievink
Copy link
Contributor

@ticki Fair enough. It's been a while since I learned Rust, and now it seems obvious to me how it works, but you're right that it might be hard to learn at first.

What's interesting, though, is that I've lately been actively avoiding generics in order to keep compile times low, so maybe using trait objects by default isn't too bad. Certainly, benchmarking is required before making this decision (which I admittedly didn't do, but I do think Breeze's <10 second compile time on core changes speaks for itself).

@ticki
Copy link
Contributor Author

ticki commented May 1, 2016

and now it seems obvious to me how it works, but you're right that it might be hard to learn at first.

Agreed. It is obvious to me as well, but I fairly often recieve PRs containing these kind of mistakes. The IRC contains lots of people asking about trait objects.

What's interesting, though, is that I've lately been actively avoiding generics in order to keep compile times low, so maybe using trait objects by default isn't too bad.

I find this rather strange. A system languages ought to focus on the runtime performance, more than the compile time performance.

@nrc nrc added the T-lang Relevant to the language team, which will review and decide on the RFC. label May 2, 2016
@peschkaj
Copy link

peschkaj commented May 2, 2016

Wouldn't it be better/more appropriate to provide better material in the book and other tutorials to ensure that newcomers to Rust are aware of the cost of specific activities? For a new user, I don't know that a dyn keyword provides clarity around the expense of dispatch than using a Box<Fn(u32) -> u32> or &Fn<u32> -> <u32>.

Further, wouldn't it also be more appropriate to add a lint to clippy and inform users what they're doing and why it's (potentially) a badness?

@ticki
Copy link
Contributor Author

ticki commented May 2, 2016

Wouldn't it be better/more appropriate to provide better material in the book and other tutorials to ensure that newcomers to Rust are aware of the cost of specific activities?

Changing the syntax would be far more effective.

For a new user, I don't know that a dyn keyword provides clarity around the expense of dispatch than using a Box<Fn(u32) -> u32> or &Fn -> .

These simply looks like normal pointers, but they're not. dyn (read: dynamic) helps this situation by making it obvious that you are not simply utilizing the pointer type, but dynamic dispatch as well.

Further, wouldn't it also be more appropriate to add a lint to clippy and inform users what they're doing and why it's (potentially) a badness?

Such a clippy lint wouldn't make much sense. You want it to show up on every case of trait objects?

@oli-obk
Copy link
Contributor

oli-obk commented May 2, 2016

Such a clippy lint wouldn't make much sense. You want it to show up on every case of trait objects?

You could lint functions that take a trait object when the implementation could be written generically.

cc https://github.com/Manishearth/rust-clippy/issues/14

@bluss
Copy link
Member

bluss commented May 2, 2016

This is how Rust is today:

“Every object compatible trait Trait has a corresponding object type called Trait”.

It sounds so confusing. From the pedagogical point of view, I very much agree that naming those two differently is a big win!

@ghost
Copy link

ghost commented Sep 2, 2016

You know, thinking about it, the whole point of including this feature as a minor version bump, is to prepare Rust programmers for when Rust 2.0 eventually exists in the long run. As an example, that's why when you guys deprecate calls or items and introduce new stuff in the std crate, is to make that migration from Rust 1.x code to Rust 2.0 code painless, as you guys plan to remove the deprecated stuff permanently.

All @ticki is advocating for is to have it there, while not absolutely mandatory to use in Rust 1. With enough compiler warnings to help further drill the awareness of dyn, obj, or whatever over a long period of time, people will catch on and will help make upgrading from 1.x to 2.x, much easier when dyn mandatory become absolutely mandatory.

The problem I see here is and what concerns me the most, is that with enough breaking changes only isolated to be introduced for Rust 2.0 only, that it will create a community split and divide between two versions of the compiler. I don't want Rust to repeat the same mistakes that Python and D did.

The reason to close this RFC based on the thought "it definitely doesn't feel to me like a minor version bump" seems like an unusual reason to decline this.

@golddranks
Copy link

The problem, or one of the problems, as I see it, is that impl Trait needs a syntax to stabilize, and if we aren't shipping this with it, I wonder if are going to ship it ever. That makes me a bit nervous.

@J-F-Liu
Copy link

J-F-Liu commented Feb 7, 2017

Use a keyword is not a good idea, I support the alternate Object<Trait>.

@ticki
Copy link
Contributor Author

ticki commented Feb 7, 2017

@J-F-Liu I'm against Object<Trait> because it doesn't make sense unless Trait is a type, which it isn't under this proposal.

@J-F-Liu
Copy link

J-F-Liu commented Feb 8, 2017

@ticki Yes, unless Trait is a type, the whole picture is this:

  • use Type, &Type, Box<Type> for concrete data types, static dispatch
  • use Trait, &Trait, Box<Trait> and generic type parameter for abstract data type, static dispatch
  • use Object<Trait> for abstract data type, dynamic dispatch

Thus to allow trait name to appear in all type positions.
fn func(x: Trait) and fn func<T: Trait>(x: T) should be equivalent.
fn func<T1: Trait, T2: Trait>(x: T1, y: T2) can be simply written as fn func(x: Trait, y: Trait).
T paramter is still needed in fn func<T: Trait>(x: T, y: T).

Benefits are:

  1. Trait can be used in function return types, think of closures and iterators
  2. I estimate 80% now generics code can be simplified

This is a great breaking change of Rust 1.x, hopefully can be adopted in Rust 2.0.

@gnzlbg
Copy link
Contributor

gnzlbg commented Feb 8, 2017

This is a great breaking change of Rust 1.x, hopefully can be adopted in Rust 2.0.

I thought this issue was already closed. IIRC the consensus was that:

  • we should have changed this before 1.0,
  • we did not do that because we did not know,
  • backwards compatibility is the single most important feature of Rust.

This particular issue can be solved mechanically, and for this reason it might encourage people to discuss a future Rust 2.x in which we can just break the world (and fix it mechanically). But this is not the only "imperfect language decision" in Rust 1.x, not all of them can be solved mechanically, and arguably we would like to solve most of them in Rust 2.x.

So while it is interesting to consider how to break the language, it doesn't really move this discussion forward. Those wanting to move this feature forward should attempt to do so in a backwards compatible way. I think that the fundamentals issues that this RFC raises (that the dynamic costs of trait objects are "too implicit") can be mostly solved in a backwards compatible way by two small incremental backwards additions to the Rust 1.x language:

  • add an explicit optional annotation for trait objects: dyn Trait/obj Trait.
  • deprecate implicit trait objects (that just use Trait).

These can be pursued in 2 small RFCs or combined into one small RFC, and will result in:

  • Trait for generics,
  • impl Trait for existentials,
  • obj Trait (or similar) for trait objects (or implicit ones, that will raise a compiler warning that can be turned off).

Whether consensus can be gathered about this changes being worth it or not, that's a different issue. But for those RFCs to be successful they should probably stay as far away from Rust 2.0 as possible.

@petrochenkov
Copy link
Contributor

we did not do that because we did not know

We knew, but nobody listened 😄
Many imperfections slipped through triage during pre-1.0 rush, but it was necessary to release 1.0 at all, I guess.

@ticki
Copy link
Contributor Author

ticki commented Feb 11, 2017

@gnzlbg Thank god for semver. In 2.0.0, we can break all the shit we want to.

@ticki
Copy link
Contributor Author

ticki commented Feb 11, 2017

My guess is that a significant portion of the 👎s given are not disagreements in syntax, but rather concerns about stability.

@petrochenkov
Copy link
Contributor

@ticki
No large scale breakage (like this one) will be possible regardless of semver if Rust actually becomes a widely used language.

@gnzlbg
Copy link
Contributor

gnzlbg commented Feb 11, 2017 via email

@golddranks
Copy link

I'd love to see the ideas from this RFC to go forward in a similar vein to the path RFC #1685 (comment) is now moving along. Yes, this is a bigger change, but I and many others think that the lack of syntactic difference between types and traits (in the context of trait objects) is a wart. It's possible to improve the situation without breaking compatibility by introducing a new more explicit syntax, utilizing rustfmt and the official documentation in the future to slowly change the the perceived "standard" and then in a long timescale, softly depreciate the old syntax with a lint. No need to break stuff.

Basically I just restated what @gnzlbg said :D Just butted in to show my support.

@gurry
Copy link

gurry commented Feb 16, 2017

One comment on the comparison with the Python 3 fiasco. By the time Python 3 came out, Python 2 was widely used in production. This is currently not true for Rust 1.x. In fact (assuming things go well), a vast majority of Rust's users are yet to come. Therefore, any fears of users shunning Rust 2.x should be tempered by this fact.

@gnzlbg
Copy link
Contributor

gnzlbg commented Feb 16, 2017

@gurry Very few were using D when they switched standard library breaking backwards compatibility. 10 years later you can still see people asking around whether D is stable yet.

@J-F-Liu J-F-Liu mentioned this pull request Feb 17, 2017
@gurry
Copy link

gurry commented Feb 17, 2017

@gnzlbd Hmm. I didn't know about that. Thanks. So you imply that D's breaking change was early enough and yet they don't have much traction today. But are we sure the said breaking change is to blame? It's also possible D didn't catch on for other reasons.

@Ixrec
Copy link
Contributor

Ixrec commented Feb 17, 2017

I still see people asking if Rust is stable enough for production use, or saying that they heard we "recently reworked our entire threading model" or some other thing from pre-1.0, so I'm not sure how much that sort of anecdata really applies. And there are definitely other reasons D didn't catch on.

I'd also like to see this keyword introduced even though we probably can't ever get rid of the keyword-less option, but until Rust 2.0 is actually likely to happen I don't feel that strongly about it.

@gnzlbg
Copy link
Contributor

gnzlbg commented Feb 17, 2017

@gurry A different way to put it is like this. Rust has many language features: memory safety, data-race freedom, generics, ... Notice that memory safety and data-race freedom are promises: we do not have a formal proof of this. Rust 1.0 added a new feature: "from now on, if your code compiles, it will compile with the latest compiler forever". This feature is also just a promise from everybody evolving Rust to everybody using it. It only works if those that use Rust trust those that evolve it.

The moment you loose that trust, that feature is gone forever and it will never come back.

Does this mean that we cannot ever break this promise? No. Imagine we find out that some other promise Rust makes does not hold, like for example, we discover that safe rust is not memory safe. That is also a promise our users rely on. If we need to break the language to fix it, we probably will. But that is a damn good reason to break the language, since otherwise our users cannot trust Rust anyways. We are trading one promise for another here, but our intention is to keep the promises we made. We would probably retain some of our users trust after doing something like this.

Now think about breaking the language to fix a minor stylistic change (like this one). We can either have this breaking change, or we can keep our users trust on stability, but we cannot have both. Do you think it is worth it to sacrifice "stability" for this minor stylistic change?

Most people think it is not worth it. Some people think it is. We are a diverse group of people, and not everybody can always get exactly what they want; we must compromise on consensus, that's just how big groups work. I actually would like something like this to eventually happen in some form, but I can empathize with the majority here, and even though I want this, I know that they are right: for the Rust language community as a whole, stability is a way more important feature to have than "more clear syntax" for feature X, in particular when even though the syntax is not the clearest that one could have, it is still clear enough in practice.

@gurry
Copy link

gurry commented Feb 17, 2017

@gnzlbg I fully appreciate what you say and do not take lightly the costs associated. I'm just trying to understand how high those costs truly are, especially at the present point of time.

Do you think it is worth it to sacrifice "stability" for this minor stylistic change?

I feel it's not a minor change. It itself and all the changes it enables further down make the language a lot easier to use and teach. Generics are everywhere in Rust. So anything that simplifies them has a major impact on the whole language. The number one complaint about Rust today is it's hard to learn. Therefore, anything that addresses that has somewhat of a disproportionate importance at this particular juncture in time IMHO.

@golddranks
Copy link

golddranks commented Feb 21, 2017

Another vague idea for soft-deprecating "plain" trait object syntax gracefully:

Have &Trait and &dyn Trait (or &obj Trait, I'm not advocating for any particular syntax here) mean the same thing, but &Trait bound would accept any of those syntaxes, whereas &dyn Trait bound would accept only the newer syntax. (This would be just a warning, not a hard error.) The idea would be that old code would work, but newer libraries (and new versions of old libraries that are releasing a backwards-incompatible versions) can mandate the new syntax, spreading the usage.

At some point, std would enable &dyn Trait bounds too, effectively "soft-deprecating" the old syntax for good.

Especially now with the discussion about having unsized rvalues and passing unsized values to functions, a new syntax would be important to have, as pointed out in #1909 (comment)

@vi
Copy link

vi commented Oct 18, 2017

Shall rust-2-breakage-wishlist label be on this repository too, not just on rust-lang/rust?

@cramertj
Copy link
Member

@vi This RFC was accepted as #2113

@vi
Copy link

vi commented Oct 18, 2017

Anyway such label would be here even more on-topic compared to rust-lang/rust.

@vi
Copy link

vi commented Oct 18, 2017

Maybe someone should edit "#2113" into the opening comment, so one can find the "take two" RFC more easily? (Google search of "rust dyn keyword" ends up here)

@cramertj
Copy link
Member

@vi Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet