-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
dyn Trait
Syntax for Trait Objects: Take 2
#2113
Conversation
👍 I believe this RFC will make it easier to write performant code because you can spot the dynamic dispatch locations more easily. Also, I think it will make learning Rust easier because it lays down a stricter difference between dynamic and static dispatch, making it easier for beginners to differentiate between the two. |
I would rather |
The first time this proposal came around it brought with it a bit of a kneejerk reaction; but it was one of those things where, the more you thought about it, the more you realized that it is absolutely, undeniably right! I will also confess that this RFC is what I had in the back of my mind the entire time as I read through the Checkpoint proposal. So now that I've thoroughly voiced my stance of approval, I must ask: Isn't it a bit early? It feels to me like it was only just yesterday that the other RFC was closed (or maybe this is just me? apparently it was a year ago...), and the Checkpoint RFC hasn't even been merged. And I think some people are still trying to recover from all of the recent discussion on the module system... I do want this to remain on the table, I just don't know if it's time to stoke that flame yet. |
@ExpHP Does have a solid point... There are already rather a lot of high-profile RFCs going around for the upcoming impl period... |
+1 for |
text/0000-dyn-trait-syntax.md
Outdated
# Explanation | ||
[guide-level-explanation]: #guide-level-explanation | ||
|
||
The functionality of `dyn Trait` is identical to today's trait object syntax. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be good to mention the impl MyTrait for dyn OtherTrait {}
syntax here, since it directly addresses the impl MyTrait for OtherTrait {}
issue in the "Motivation" section.
I definitely believe the correct grammar is |
text/0000-dyn-trait-syntax.md
Outdated
|
||
- Code that uses trait objects becomes slightly more verbose. | ||
|
||
- `&dyn Trait` might give the impression that `&dyn` is a third type of reference alongside `&` and `&mut`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just make it &mut dyn Trait
then?
|
👍 from me. Obviously, this has been stewing for some time. I think there is ample evidence that using I have also come around to the choice of keyword. One question that might be worth discussing a bit: the terminology "trait object" is something that the compiler (and, I believe, some of the documentation) uses. For example, we say that traits are "object safe" when we can use them to make vtables. The RFC makes a rather brief argument against that terminology -- essentially, there are too many differences between "object-oriented systems" and "trait objects" for that to be a helpful intuition. I am not sure I fully agree with the argument, but regardless it seems like we should try to align our "English" terminology with the keyword we choose. If that is to be The RFC also opens up some interesting areas for the future. Obviously these would be future RFCs, but I did want to highlight two things I've been thinking about: The first is the possibility of repurposing "bare trait" to mean The second is the idea of permitting explicit erasure on type arguments to control monomorphization. For example, one might be able to do: fn foo<dyn T: Debug>(x: &T) {
...
} This would indicate a Java-like compilation strategy, where we all values of type This latter use seems to indicate the value of making dynamic dispatch opt-in for types, since it allows us to use the same opt-in elsewhere. |
Do note that a trait object is a |
"Dynamic trait types/values". "Dynamic traits" sounds like a feature of the traits themselves, rather than just of the types/values. Of course, I'm so used to "object types" that I don't see the problem either ;). The other related term is "object safety". It seems like the term would become something like " |
I'm personally not necessarily against this RFC but not necessarily for this RFC. My opinion purely stems from compile times in Rust today, and let me elaborate! Two of the headline sections of the motivation section in this RFC right now are:
And these are correct! It's pretty rare to see trait objects today and they are indeed factually slower than inlined methods in any possible microbenchmark. My thinking, though, is that this comes at a very real cost to compile times in the compiler. The number one source of compile time slowness today is, I believe, due to monomorphization. This is because whenever you call a generic function we have to generate LLVM IR for that function call (each time it's called) and LLVM also has to optimize it and generate code for it. On small scales this doesn't matter too much but I'd like to talk about Rust code "in the large" for a moment. I'd like to start out with some data I've collected to try to confirm this hypothesis. I wanted to analyze the top thousand crates on crates.io and evaluate how much code the compiler translates specifically for that one crate and specifically for upstream crates. That is, if you call a generic function in the standard library, like For each crate on crates.io I emitted the IR for the final crate. For each function in this final crate I classified each LLVM function as "from dependencies" or "local" depending on the debuginfo (looking at the filename). I then counted the total number of lines of LLVM IR to attribute to "from dependencies" and "local". And with all that I got the following graph. The x-axis here is size of a crate's dependency graph (all crates involved). The y-axis is the ratio of "upstream code" to "local code", plotted on a logarithmic scale. That is, any data point greater than 1 means that you're generating more upstream code than local code. This graph confirms to me, for example, that we have an exponential code growth problem. As you pick up more dependencies, the amount of code that just the final crate has to work with is growing exponentially. Way over half the crates on crates.io have over half their compile time to "blame" to upstream crates! So basically what I wanted to get at is that rustc compile times are slow, and I believe the serious contributing factors are:
So with this mindset, I'm personally sort of opposed to discouraging the number one way to combat this problem, trait objects. I believe that this RFC would erase almost all usage of trait objects casually in the ecosystem, as it's more ergonomic to take a generic than it is to take a trait object. I personally view this as the wrong default. I'd hypothesize that for 99% of code you don't actually need the performance benefits of 100% monomorphized code. Sure we need An example of this is the On plausible scenario, though, is that instead of fn foo<F: FnMut()>(f: F) { /* ... */ }
foo(|| ...);
// vs
fn foo(f: &mut FnMut()) { /* ... */ }
foo(&mut || ...); One of these is clearly more ergonomic for the caller! Especially once we add All in all I don't really intend for this to be a rant, but rather my own personal rationale for why making trait objects worse in their current syntax to be a step in the wrong direction towards improving Rust's compile times. I think that Rust programmers lean on virtual dispatch far too rarely and this comes at a harm to Rust's compile times. Now the problem of not using virtual dispatch goes much more deeper than the surface syntax. Types aren't object safe, they're tough to store in a struct, you deal with lifetimes quicker, etc. My point though is that we keep pushing into the world of hiding virtual dispatch as much as possible so even possible usages of trait objects today may shy away with the syntax of I think all I'm really getting at is that I'd personally like to see this as an explicitly mentioned drawback of the RFC. This won't make Rust's compile times 10x slower or anything, but it, in my mind, solidifies trait objects as a corner of the language only experts use. I'd personally prefer to try to push the needle in the other directoin (making trait objects easier to use), but I realize I'm very likely to be in the minority! I do think that @nikomatsakis's thoughts about |
@alexcrichton that's an awesome comment! Compile times are in fact my top one concern with the Rust compiler. But for me compile times are not important enough to justify making Rust a slower language. That being said, I do think that generics in public APIs of a library should be encouraged only where they are actually needed. |
👍 I really like that this improves the " |
@alexcrichton I agree that dynamic dispatch is unfairly maligned, and I don't think "making dynamic dispatch worse" is a motivation for this RFC (the compelling motivations to me are those to do with learnability). I'd like in the future to make trait objects more ergonomic to work with, which I think has less to do with their type syntax and more to do with how object safety feels restrictive & how you have to box them to deal with them by value. |
Can you explain why you think this RFC makes it more ergonomic to use generics than to use trait objects? |
@withoutboats a very good point! We talked a bit about this last night as well, and the learnability aspect really resonates with me as well. I think I've over-rotated on the "favors a feature that ... is sometimes slower" point in the motivation of this RFC too much. I think you've got a very good point that the syntax may not be much of a deterrent here, but rather all of the papercuts with trait objects (object safety, caller sigils, etc). I think I just got a bit carried away with the "cost of monomorphization" semi-rant! @cramertj I personally at least today see the trait object and generic syntax as "roughly equivalent" on the library-author side of things, for example: fn foo<F: FnMut()>(f: F) { /* ... */ }
fn foo(f: &mut FnMut()) { /* ... */ } In that sense I'd subjectively view the fn foo<F: FnMut()>(f: F) { /* ... */ }
fn foo(f: &mut dyn FnMut()) { /* ... */ } instead of fn foo(f: impl FnMut()) { /* ... */ }
fn foo(f: &mut dyn FnMut()) { /* ... */ } I agree that the latter (using Regardless though discussion with @withoutboats last night really clarified for me that the heart of this proposal is about the learnability aspect of dynamic dispatch today. That definitely makes sense to me, and attempting to pursue, in parallel, proposals to make |
Has anyone considered using |
I'd use |
Almost all compiler optimizations rely on inlining, but as you say, inlining destroys separate compilation. That is, monomorphization makes code fast and compile times exponentially slow, while trait objects make code slow and compile times fast (linear). Obviously, this is not black & white, and in many cases the opposite is true, but in general, I think we can agree that this holds. What I do not agree with is that dynamic dispatch makes it a better default. IMO it is way way easier to make code that uses monomorphization everywhere compile fast (by caching, limiting inlining, outlining, ...) than to make code that uses dynamic dispatch run fast (whole-program link-time optimization, devirtualization, "just use ThinLTO and hope for the best", ...) . So sure, monomorphization increases code-size, compile-times, etc. and this is a known problem in every programming language out there. Are we already doing all of the following?:
IMO, pursuing all of these approaches to speed up the compilation of generics by trading off performance is way easier than pursuing even the most trivial optimizations for trait objects. OTOH if we switch to dynamic dispatch "by default", and performance becomes an issue, there isn't really a lot that we can do to improve on it. This is why I think that what you propose, that is, pursuing trait objects by default, has a way higher risk long term than what we currently have. |
Sure. Type-checking is only done once.
We have byte-code ("MIR"), but because of how people write it + the absence of MIR inlining, it takes some amount of time to monomorphize.
People don't tend to use
The same type (e.g. literally
No, but again complex types. |
However, I think a main part of the "exponential" problem occurs because of "code at scale", e.g. |
@arielb1 When I mentioned caching, I was referring to do so across all crates. E.g. if a crate A exposes a generic function
When private crate types are involved, then obviously this doesn't help, because the monomorphization / code generation happens already at crate granularity, and other crates cannot name private types, so they cannot ask for the same monomorphizations. Do we have statistics for this? That is, statistics about the number of unique monomorphizations (asked once, required once) vs repeated monomorphizations (asked by many crates) when building large projects like servo? These numbers might be interesting. A common compile-time optimization in C++ is to factor out the part of the code that does not need to know about the exact type into a base-class. But these are more "library-specific" optimizations to reduce compile-times, I was only talking about general optimizations that don't require users to change any code.
Sure, and if the types in the child crates are different, then that's the way it is (unless we start trying to type-erase generics in debug builds). But if all child crates instantiate it with the same types, right now, the code is monomorphized N times, instead of just once.
What do you mean by "how people write it" ? |
Rust code has lots of calls to tiny helper functions like |
I presume this means once per binary, not per crate in the binary.
I'd think compiler optimizations could often do this under the hood by creating some form of light weight internal function call with a light weight internal trait object. An inlined monomorphized function
Or maybe you should write :
|
Updated the RFC to specify that bare trait syntax would not be removed completely in the next epoch, but instead become a deny-by-default lint, in accordance with the latest updates to the epochs RFC as summarized here. Because that RFC does allow us to "leverage the corresponding lint setting to produce error messages as if the feature were removed entirely," all of the benefits of this RFC remain intact. |
I've set up a video meeting to discuss |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
No bringing back sigils, please. :)
I'm still not sure whether this RFC is worth accepting. For all you people out there hoping that this will turn Given the nature of this change, I also question whether it isn't premature to accept this given that the epochs RFC has yet to be accepted. There are plenty of people who look at the bevy of RFCs being accepted recently, and the dozen more standing by to be accepted, and wonder if the impending impl period isn't leading to premature decisions. It erodes trust in our decision-making processes. Let's make sure we're not making such changes frivolously. |
I in fact hope that |
I agree with @est31 on the last part, but bare trait syntax could repurposed for cases like impl Trait1 for Trait2 { ... } Which could be syntactic sugar for: impl<T: Trait2> Trait1 for T { ... } This would eliminate confusion about what the former is really doing and why it's giving such confusing errors for non object-safe traits. Appologies if syntax is wring, on mobile. |
@tmzt In addition, what I like best about @est31's proposal, is that how the code reads, is a lot more accurate to what is going on.
|
We should squander good distinctive syntaxes duplicating existing functionality because they create confusion and we've interesting things like inherent |
Since impl Trait1 for impl Trait2 { ... } And then just use |
I'm strongly opposed to |
Re:
If we ever have bare trait names used as types mean impl Trait, that approach will no longer work. However, this RFC doesn't propose that, so I guess this will only be a problem if someone ever suggests (in a future epoch) to allow bare traits for impl Trait. |
Would using Otherwise I prefer |
@WaDelma soon, trait objects won't have to be boxed in many situations (unsized rvalues RFC). |
The final comment period is now complete. |
This RFC has been merged! For those who haven't followed the thread closely, the main change since the RFC was opened was to relax the rollout story, employing a lint for bare trait syntax, and only raising that lint to deny-by-default in the next epoch. This is as per the finalized epoch policy. With respect to bikeshedding on the In any case, discussion will now continue on the dedicated tracking issue. Thanks all for the great discussion, and @Ixrec for writing the RFC! |
Introduce a new
dyn Trait
syntax for trait objects using a contextualdyn
keyword, and deprecate "bare trait" syntax for trait objects. In a future checkpoint,dyn
will become a proper keyword and bare trait syntax will be removed.Rendered
The previous RFC on this topic is: #1603
The RFC that describes checkpoints is: #2052