-
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
Add RFC undo-universal-impl-trait. #2444
Conversation
Aren't universal impl trait stable since 1.26 ? This would be a breaking change. |
I guess you could remove it as part of Rust 2018. That should work |
I like the idea. |
It would be breaking change. |
The fn add_self<T: Add<Output = O> + Copy, O: Sized>(x: T) -> impl Sized {
x + x
} We can make a non-impl trait version that is even better, since we can specify that the fn add_self<T: Add<Output = O> + Copy, O>(x: T) -> O {
x + x
} We can mix this with impl trait for what is my preferred version fn add_self<O>(x: impl Add<Output=O> + Copy) -> O {
x + x
} What I get from this whole line of reasoning though is that universal impl trait encourages better code. It makes you realize what types need to be equal, and what ones don't. |
I'm very much in favor of removing
(Sorry if this sounds like a rant, I just feel very strongly about this :) |
I completely agree with this PR. I didn't understand why we had two superfluous syntaxes for the same thing (although impl Trait is less useful as you can't turbofish it). It's also really really confusing to use the same syntax for two subtly (and completely) different bounds -- already some (I used to be until I did some reading) may be confused as to what the differences of universal and existential quantification are, thus making them share syntax is even worse for clarity. |
Does anybody actually think in terms of 'caller' vs 'callee' chosen types? Are we really confusing people aware of the type theory involved for more than a few moments?
|
What we see here is a huge failure of public review process. Simply put, |
@logannc Yes, we are confusing people who understand what the difference between universal impl Trait and existential impl Trait. I researched the topic extensively to understand it, and after a few hours I was still sort of uneasy as to why we had it do two perpendicular things.
No, this distinction does not go away. As a universal bound there are multiple types you can call the method on (i.e |
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.
This RFC strikes me as in bad faith. This feature had an RFC open for 2 months before it was merged. It's been available on nightly for many more months than that. This mostly seems like a rehashing of arguments that were already made and considered during that time period.
While we should certainly be allowed to undo things that turn out to be mistakes, I don't think it's in the spirit of this community to be opening PRs to undo features you dislike within weeks of their stabilization.
The biggest question I'd ask is what's changed since the RFC to justify this? The majority of this seems to be "I saw the rationale behind this feature and I disagree", without presenting any new information that would change the decision.
`impl Trait` carries its own semantics and because anonymization in return type is not a trivial | ||
feature people will use without actually wanting it. | ||
|
||
Also, people are used to generics. People coming from C++, C#, Java etc. actually know about |
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.
You've followed
This argument is twisted along the lines of subjective opinion about learnability
With an equally subjective argument.
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.
People come to Rust from languages other than C++, C#, and Java. Not to mention that the entire point of impl Trait
in argument position is that it works much closer to Java/C#. In java, you don't write void <T extends SomeInterface> method(T arg)
, you write void method(SomeInterface arg)
.
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.
Interfaces in Java are not akin to universal quantification, because those are dynamic dispatched variables. The equivalent in Rust would be &Trait
or dyn Trait
.
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.
Yup, but the entire point is that you generally don't care about the tradeoffs between a monomorphised function and a dynamically dispatched one. Similarly, you generally don't need to care about the differences between returning Box<Foo>
and impl Foo
. This is a "do what I mean" feature. Similar to lifetime elision, it hides details that you usually don't care about. If you end up in a situation where you do need to care about it (T: Add<Output = T>
is a good example), or want to care about it (you're in a situation where binary size is very important), then you learn the underlying mechanisms.
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.
You generally
Are you sure about that? :)
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.
@sgrif
Wouldn't the actual equivalent of
void foo(SomeInterface arg)
be
fn foo(arg: Rc<RefCell<dyn SomeTrait>>)
?
Because the instance is GC'ed and mutable.
So when translating Java code (or thinking) to Rust, one wouldn't just be able to write fn foo(arg: impl SomeTrait)
instead of the java method anyway, because that would be different semantics (it takes ownership and can't mutate). One would have to re-architect the code in the translation process to use references/lifetimes and to please the borrow checker anyway.
So impl Trait
for args isn't really the equivalent (or what one wants) in most cases when translating Java/C# code.
way to use generics (`impl Trait` in argument position is a use of hidden generics, that is, | ||
anonymized type variables with explicit trait bounds). For people who don’t come from such | ||
languages or for completely newcomers, we are assuming that generics are too hard to understand at | ||
first but people will eventually need to learn them, as `impl Trait` in argument position is |
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.
This is no different than lifetime elision. fn foo(&self)
is strictly less powerful than fn foo<'a>(&'a self)
.
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.
How so?
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.
fn foo(&self, x: &str) -> &str
vs fn foo<'a, 'b>(&'a self, x: &'b str) -> &'b str
. You can use the second in more places, e.g. with 'a=lifetime of some object
and 'b = 'static
(or just the lifetime of some longer lived object).
This comes up in real code on (rare) occasion.
fn foo<T>(a: T, b: T, c: impl Debug) where T: Add<Output = T> + Copy | ||
``` | ||
|
||
This function has three type variables to substitute to be completely monomorphized. However, it’s |
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.
I think you meant two type variables
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.
Is the number of variables being monomorphised something people frequently care about? Is it a case we need to optimize for over readability?
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.
Yep, thanks for noticing. For the number of variables, I actually care about that, because it gives me hints about code generation and which variables I can have control over – via turbofishing, for instances. It’s way harder with impl Trait
because you’re not allowed to turbofish anymore. 😞
Now consider: | ||
|
||
``` | ||
fn add_self(x: impl Add<Output = impl Copy> + Copy) |
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.
This signature is not equivalent. The only output type you could have here is impl Copy
or ()
fn add_self(x: impl Add<Output = impl Copy> + Copy) | ||
``` | ||
|
||
You can see the duplication here and the overwhelming confusion that both the `impl Trait` will |
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.
This argument is entirely subjective. I personally don't find this confusing. I'm taking "some type that can be added and copied. The result of the addition is some type that can be copied".
"overwhelming confusion" feels like unnecessary rhetoric to me.
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.
You’re right, it’s too much subjective. I’m removing it.
|
||
### Summary of arguments | ||
|
||
This document has shown that `impl Trait` in argument position is confusing for newcomers and |
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.
This document hasn't really shown anything from newcomers. It's pretty hard for us to gauge how this affects newcomers at this point, since it does not appear in the book or anywhere in the documentation.
Instead this document seems to be claiming every other argument is subjective, but this one is somehow objective.
Perhaps the source of confusion is the fact that it's a brand new, undocumented feature that people are still just starting to use.
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.
I’ve linked a few links in the top of the document. Also, reactions and the fact it’s been so opinionated is somehow a hint that there’s a massive confusion, right?
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.
I mean, personally I was confused as to why we had the same syntax for the same thing, but that's just me I guess.
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’s not just you. All of this is very confusing and opinionated.
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.
Also, reactions and the fact it’s been so opinionated is somehow a hint that there’s a massive confusion, right?
People having strong opinions does not mean they're confused.
|
||
## Why is `impl Trait` in argument position wrong? | ||
|
||
People out there didn’t really realize what was going on. Complains and confusions on IRC, reddit |
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.
This feels like a straw man to me. The feature is not yet documented. There is always going to be confusion until that gets remedied. There's also a lot of confusion around how reborrowing works (another undocumented feature), and that feature's been around for much longer.
because they will maintain and contribute to codebases they haven’t read nor written code for | ||
before. | ||
- Turbofishing is impossible with `impl Trait`, forcing you to use type ascription in other | ||
places. |
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.
This is fixable.
Technically, nothing prevents supporting turbofish and making arg position impl Trait
purely a sugar for named type parameters.
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's slightly more arcane, though. Often when doing something like fn x<A: TraitA, B: TraitB<A>>(b: B)
I write it with the thing the 'main' generic depends on first (I don't know if you are allowed to put it second, but either way it feels 'wrong' to do it in reverse). This might not be so with impl Trait turbofishing, or at any rate is more difficult to figure out.
Excuse the stupid question. How is impl Trait in argument position any different than say.. C# where a function or method takes an interface instead of a concrete type? Can someone explain that to me. Because in C# I don't feel like its confusing at all. I'm still learning Rust but when I saw impl Trait in argument position I just equated it to doing the same thing I would do in C# with interfaces. |
I added a bunch of comments from here. Thank you all for such massive participation. |
In terms of program behavior it's equivalent to passing an interface in C#. Arguably the closer equivalent would be taking a "trait object", but this is something that only matters if you care about very specific performance trade offs that you don't normally need to care about.
That was the goal behind the feature. |
@xgalaxy to me, it’s different because of dynamic dispatch vs. static dispatch. The Rust equivalent would be:
Both those functions are completely monomorphic, not polymorphic. |
Perhaps this is the wrong time and place to ask this, but what is turbofishing and why does it matter that it is impossible with |
@bowbahdoe turbofishing is a way to force monomorphization of a function by providing it with the type variables substituted. You already have done it for sure. See:
You cannot do it with |
@sgrif Are you referring to And being available in the nightly means nothing. There are a lot of experimental and incomplete features that can be turned on via feature gate and may go away at any time. Just look at non-existant trait aliases for example. |
@phaazon There's an important detail that you're leaving out, which is that turbofishing is almost never required for type variables used in an argument position. The compiler can infer it from the type of the argument you're passing. The only time I've ever needed to turbofish an argument parameter is when passing a generic In fact, I hope the way this ends up getting resolved is to allow type parameters to be explicitly passed, but no way to explicitly state the type of
The type of
which would remove the pointless |
@sgrif correct me if I'm not right, but your code fn debug_query<DB: Backend>(query: &impl QueryFragment<DB>) -> DebugQuery<impl QueryFragment<DB>, DB>
T: QueryFragment<DB>, is equivalent of fn debug_query<DB, T, U>(query: &T) -> DebugQuery<U, DB>
where
DB: Backend,
T: QueryFragment<DB>,
U: QueryFragment<DB> Which obviously is not the same thing as fn debug_query<DB, T>(query: &T) -> DebugQuery<T, DB>
where
DB: Backend,
T: QueryFragment<DB>, So you probably just planted a bug in the oneliner. As pointed out below, it's a bit incorrect, there is no 3rd type parameter. However, it shows that U is not related to T which may be unwanted. |
@logannc I think you're both right and wrong. You're right, it's simply a stand-in for "some type that implements this trait, the compiler will figure out the rest." However, it does not exist in a vacuum. It must interact with the existing type system. And in that, it fails. Badly. Take for example, this function signature:
Now, the user wants to make the function more robust by adding a Result type. They want to return the original data, because they're passing it in by value somewhere. How? They might try this:
But now this does something different. The compiler forgets the input type. So if the user inputs a Vec and expects the same thing out, they might try to debug it with {:?}. But they will get an error: Very basic assumptions about how the user expects types to be propagated are lost BECAUSE we have a type system that is strict- the compiler is not going to "figure it out at the call site", it's just going to follow the exact definition in the function signature. There is no way to write this without named generics. |
Another example from Diesel that would be improved by
in typical usage, This case is a little more interesting, since we can remove the
which would change the invocation to |
@Pzixel that is incorrect. |
@sgrif Yes, but how do you teach that to a newbie? The difference here goes back to the caller vs callee distinction. You need to know about that distinction to be able to understand why those two functions are different.
) |
Well, could you elaborate a while then, please? IIRC each input fn foo(value: &impl std::fmt::Debug) -> impl std::fmt::Debug {
42
}
fn main() {
foo(&"42");
} input type is not tied to the output one in any way. |
@Pzixel It's equivalent to |
@Pzixel only the input In your example:
This works with type parameters (if you declare the returning type to be u8 or something like that, to resolve the ambiguous integer type). It does NOT work with impl Trait - at no point does the function guarantee it returns the same output type for those two different input types. |
@Phlosioneer I think you're over-emphasizing the importance of knowing/caring about callee chosen vs caller chosen. A better comparison to make would be the difference between |
@sgrif IIRC, in the D language, turbofish allows omitting type params (from the end), they don't have to be substituted by So then you'd put all the inferrable type params at the end of the |
@Boscop Yup, that's a direction we could go as well. As with all things, it's a tradeoff. |
Yes, you're right, thank you. However, I still think that it leads to confusion. There should be only exact one way to do some thing. The main reason against |
Are you of the opinion that lifetime elision should also be removed? These two features have similar uses and drawbacks. You also can't provide a lifetime explicitly via turbofish when lifetime elision is being used. Sometimes you need to explicitly name them (either to show equivalence or some relationship between multiple lifetimes), but it's pretty rare. Same for I think it's important that we have a way to drop down to a lower level, either because you need to use a turbofish (extremely rare for argument parameters) or because you need to name a type. But this feature gives us a great parity between |
No, lifetime elision is fine, because it's the only way to specify object lifetimes. There is no "easier but more fragile syntax" that allows you to write the very same thing but using a bit less characters. I love In a nutshell: |
We are going to take the unusual step of immediately closing and locking this RFC. While there are clearly a lot of feelings on all sides of the issue, the fact is that this feature has shipped on the stable channel and is not going to be removed. This is both because of our basic stability guarantees but also because the decision was made in good faith and pursuant to our process, and we stand by that. After multiple years of RFC and tracking issue discussions (the first one was RFC #105 in 2014!), the Rust Language Design Team ultimately reached a decision to ship this feature. This decision was not reached lightly. We discussed in depth a number of alternatives, including limiting To be clear: we understand that there are downsides to this feature, and that some people find those downsides concerning. All of us care deeply about Rust, and it can be distressing to see people in power moving things in a direction you dislike. But, at the end of the day, we have to be able to make — and stick with — decisions, striking a balance between long-running feedback and shipping. Rust 2018 will ship with -- @nikomatsakis and @aturon, on behalf of the Rust Language and Core Teams |
Rendered
Closed and locked; see comment from leadership below.