-
Notifications
You must be signed in to change notification settings - Fork 1.7k
RFC: Natural Method Disambiguation #3913
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
base: master
Are you sure you want to change the base?
Conversation
|
This is a direct follow-up to #3908, with a few key differences:
Therefore, I have decided to start with a clean slate. |
|
Afaik we cannot justify two syntaxes for UFC, and the proposed new syntax does not improve over the existing UFC for traits, but if we need clear access to inherent methods then roughly |
why not? we already have multiple different syntaxes for function calls:
I disagree. It's much more ergonomic to read and write when you have a method chain since you don't have to deal with all the nested parenthesis and reading the functions from the inside out.
I think we also want some syntax for only accessing inherent methods when you're not in a method chain, e.g. because your functions don't have a |
|
If you don't mind both of you, could you start an inline comment thread for this point?
|
I guess we should ping @burdges for this. |
teor2345
left a comment
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.
Some suggestions, mainly alternatives and prior art
|
|
||
| * **Why Angle Brackets for Trait Method Calls?** | ||
| * `value.Trait::method` looks like there is something called `Trait` inside the `value` while `Trait` is coming from the scope of the call. | ||
| * `value.<Trait>::method` aligns with Rust's existing use of angle brackets for type-related disambiguation (like UFCS `<Type as Trait>::method`). |
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 it worth explicitly listing an extended UFCS syntax as an alternative?
For example:
value.<Type as Type>::methodvalue.<Type as Self>::methodvalue.<Self as NotATrait>::method(if chosen, we can bikeshed the spelling ofNotATraitlater) as an alternative tovalue.Self::methodorvalue.<Self>::method
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 would you add extra characters that don't add any information?
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'm not saying it's a better alternative to what's currently in the RFC. It might be worth listing them to show they have been considered. (And to cover future questions/suggestions like this.)
One possible reason is consistency, but I don't think it's a good enough reason to justify the extra characters in this specific situation.
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.
Motivation
Method chain break
Currently, Rust's "Fully Qualified Syntax" (UFCS), e.g.,
<Type as Trait>::method(&obj)(or less commonlyTrait::method(&obj)), is the main mechanism to disambiguate method calls between inherent implementations and traits, or between multiple traits.
It is worth noting that the proposed syntax is essentially a minor reordering that shortens the construct by removing
Type asand the&/&mutoperators, which carry no specific disambiguation information in this context.
I can just refer to this in such cases
| ## Prior art | ||
| [prior-art]: #prior-art | ||
|
|
||
| * **C++**: Allows explicit qualification of method calls using `obj.Base::method()`. |
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.
Swift has anti-disambiguation prior art, but it still might be worth mentioning.
Swift bans any type annotations on method calls:
Which causes unfortunate consequences in some cases, like having to pass a type as a function parameter:
C# (and I think also C++) can disambiguate interfaces using type casts: ((Base)(obj)).method()
C# extensions are more similar to Rust traits, but I couldn't find specific documentation about disambiguating them.
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.
| * In `impl` blocks, we can apply `obj.Self` to objects that do not have the type named `Self` in that block. `obj.<Self>` would look like we are trying to apply a method of one type to an object of another type even if they happen to be the same. | ||
|
|
||
| * Despite being technically feasible for the compiler to parse, `obj.<Self>` would appear clunky and unidiomatic. | ||
|
|
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.
Another alternative or future possibility is:
- not importing the trait, so you can't accidentally call its methods
- add a clippy lint or compiler warning for trait methods that will be called if the inherent method is renamed or removed, to help clean up trait imports
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.
The second is a good one
| > [!NOTE] | ||
| > Since `U: Copy` lacks `+ Display` bound required by the inherent implementation, the inherent method is not applicable within this context, causing the compiler to resolve to the trait method silently. | ||
|
|
||
| You would also get the same undesirable behavior in another case. You could rename `something` in `SomeThing`'s impl block and forget to rename it in the `SomeTrait`'s impl block |
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 assume this code causes infinite stack recursion at runtime?
There's already a compiler check for simple infinite recursion. But it's hard to lint against more complex cases of silent receiver changes.
| You would also get the same undesirable behavior in another case. You could rename `something` in `SomeThing`'s impl block and forget to rename it in the `SomeTrait`'s impl block | |
| You would also get the same undesirable behavior in another case. You could rename `something` in `SomeThing`'s impl block and forget to rename it in the `SomeTrait`'s impl block, causing infinite recursion at runtime |
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.
"The same undesirable behavior" already refers to infinite recursion at runtime which I showed above. If it still doesn't seem redundant to you even keeping that in mind, then okay, I'll go ahead and commit this.
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 worth specifically saying that the undesirable behaviour is only detected at runtime, and that it's an infinite recursion terminating in a stack overflow. Silent errors like this are a stronger justification for language changes.
They're implied by the example code and outputs, but a busy/casual reader might miss it.
As you say, it probably belongs further up, near "the code compiles successfully and prints"
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.
Note
Since U: Copy lacks + Display bound required by the inherent implementation, the inherent method is not applicable within this context, causing the compiler to resolve to the trait method silently which results in infinite recursion at runtime.
You would also get infinite recursion in another case. You could rename something in SomeThing's impl block and forget to rename it in the SomeTrait's impl block
Better?
|
currently as a free function let a: u32 = Default::default(); // ok, we get `<u32 as Default>::default()`
let b: u32 = <Default>::default(); // error, interpreted as `<dyn Default>::default()`so I find it very strange the proposed "natural" syntax to be |
Using this, you would always have to write 5 extra characters: |
This RFC proposes two new forms of method call syntax for method name disambiguation that keep the receiver on the left and preserve chaining. In doing so, it takes a step towards making implicit selection between trait methods and inherent implementations optional.
An example of a chain of calls using proposed syntax:
obj .Self::chain() .<Trait1>::of() .<Trait2>::method() .<Trait1>::calls();Important
When responding to RFCs, try to use inline review comments (it is possible to leave an inline review comment for the entire file at the top) instead of direct comments for normal comments and keep normal comments for procedural matters like starting FCPs.
This keeps the discussion more organized.
Rendered