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 support for use Trait::func
#3591
base: master
Are you sure you want to change the base?
Conversation
From The Rust Programming Language section 7.4:
We have been encouraged for a long time to always import the parent module when calling the function to distinguish locally defined functions and imported functions. However, the syntax sugar suggested in this RFC contradicts to this convention. I think it is more appropriate to discuss this convention in the RFC to make it more clear how we should use this feature. :) |
I'd like to reiterate here what I said on #1995 Many libraries define free functions for constructors to mathy types, like vectors. Some examples: It would be nice if instead of having to create wrapper functions, the user (or the lib author with This is some additional motivation that could go into the motivation section. |
That being said, I noticed that this RFC doesn't talk about importing inherent methods, which I think should be addressed. |
I discussed that in the future work section. |
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 RFC text uses the term “method” where it should be using the term “associated function”.
- An associated function is a function defined in an
impl
or declared in atrait
. (reference) - A method is an associated function that has a
self
parameter (and therefore may be used in.func_name()
method-call syntax). (reference)
I recommend that this be corrected to avoid creating confusion. In particular, the central use case of this feature is importing associated functions that are less concise to call because they are not methods, such as Default::default
.
text/0000-import-trait-methods.md
Outdated
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 surprised no one has mentioned generic traits yet. We would probably want to reject such kinds of imports but that should be stated explicitly in this RFC.
Consider:
use Trait::function;
trait Trait<'a, T, const N: usize> {
fn function<'b, U, const M: usize>();
}
Unless we somehow concatenated the two list of generic parameters and where clauses which probably would have questionable semantics and other unforeseen consequences, there would be no way to explicitly specify the trait generics given only the associated item.
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.
Well, maybe there's no need to reject those kinds of imports but I feel like they're a bit footgun-y but maybe that's just me. Since function
would basically be <_ as Trait<'_, _, _>>::function
, I'm just wondering in how many use cases / scenarios inference would actually succeed in practice and in how many there would be one param that can't inferred requiring users to switch from an imported associated function back to a more qualified path 🤔 Could be a non-issue though, I dunno.
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.
So, with the desugaring I specify, it is unambiguous what should happen in this situation, which is exactly as you state. The compiler will try to infer all trait parameters, and if it can't it will throw an "unable to infer error". Then you just switch to the fully qualified syntax. This is exactly the recommendation we make for ambiguous inference right now.
text/0000-import-trait-methods.md
Outdated
# Summary | ||
[summary]: #summary | ||
|
||
Allow importing methods from traits and then using them like regular functions. |
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.
If this PR only plans on adding support for importing associated functions, it should imo explicitly state explicitly that it excludes associated constants and types and add a short explainer for why that's the case. Note that implementing this feature for associated constants would probably be easier to do than for associated functions, so there's not much reason to exclude them, imho.
(Importing associated types on the other hand should probably be forbidden. Unless we reify the implicit Self
type parameter (i.e., turn it into an explicit parameter), such an associated type Ty
would be equivalent to <_ as Trait>::Ty
which is impossible to solve since associated types are not injective in general.)
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 only ever seen this feature mentioned in with associated funcs. Writing in associated consts shouldn't be too hard, but I only want to do it if it is wanted.
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.
@obsgolem If you think associated constants would be straightforward to add to the RFC, please do go ahead and add it. If you think it'd be a challenge to add to the RFC, please mention it in future work instead.
use Trait::method
use Trait::func
You cannot import a parent trait associated function from a sub-trait: | ||
|
||
```rust | ||
use num_traits::float::Float::zero; // Error: try `use num_traits::identities::Zero::zero` instead. |
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 seems like a notable limitation. We've talked about adding a compatibility mechanism that allows implementing supertrait methods via the subtrait, such that it would then be possible to extract a method from a trait into a supertrait without breaking compatibility with any user of the trait. This limitation would prevent that.
So, at the very least, if we add such a mechanism we'll have to fix this limitation at that time as well. Could you add a note to future work to that effect? "If we add a compatibility mechanism to implement a supertrait method when implementing its subtrait, without having to separately implement the supertrait (such that a new supertrait can be extracted from a trait without breaking compatibility), we would also need to lift the limitation on using a supertrait method via a subtrait."
```rust | ||
use Trait::func as m; | ||
``` | ||
occurs, a new item `m` is made available in the value namespace of the current module. Any attempts to call this item are treated calling the associated function explicitly qualified. As always, the `as` qualifier is optional, in which case the name of the new item is identical with the name of the associated function in the trait. In other words, the example: |
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.
occurs, a new item `m` is made available in the value namespace of the current module. Any attempts to call this item are treated calling the associated function explicitly qualified. As always, the `as` qualifier is optional, in which case the name of the new item is identical with the name of the associated function in the trait. In other words, the example: | |
occurs, a new item `m` is made available in the value namespace of the current module. Any attempts to call this item are treated as calling the associated function explicitly qualified. As always, the `as` qualifier is optional, in which case the name of the new item is identical with the name of the associated function in the trait. In other words, the example: |
Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members: Concerns:
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. |
|
||
Additionally, the syntax | ||
```rust | ||
use some_module::Trait::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.
This seems badly phrased to me, use foo::self
is generally disallowed.
error[E0429]:
self
imports are only allowed within a { } list
(its nitpicky I know, but it should still state use some_module::Trait::{self};
instead and rephrase it in terms of whats already allowed with trailing self
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.
Perhaps jump straight to a use some_module::Trait::{self, method};
example, since that's the goal of this 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.
Actually, on second though, that is already allowed
trait T {}
mod module {
use crate::T::{self};
}
compiles today as expected. So yes, it would make more sense to just jump straight to the use some_module::Trait::{self, method};
example and drop the rest.
|
||
Because of this context sensitivity, we should allow developers to choose when removing the extra context makes sense for their codebase. | ||
|
||
Another drawback mentioned during review for this RFC was that this adds more complication to the name resolution rules. On an implementation side, I am assured that this feature is straightforward to implement. From a user perspective, the name lookup rules for the function name are exactly the same as those used to look up any other function name. The lookup rules used to resolve the `impl` are also exactly the same ones used for non-fully qualified trait function calls. There is no fundamentally new kind of lookup happening here, just a remixing of existing lookup rules. |
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.
On an implementation side, I am assured that this feature is straightforward to implement.
Note that this may be the case for rustc, but not necessarily for alternative implementations. Wearing my rust-analyzer hat I'd expect this to be doable as well, but a bit annoying. Not saying this is a blocker (it's not impossible to implement) but mainly to keep that in mind until we attempt to implement it in r-a.
Rendered
This feature fully supplants rust-lang/rust#73001, allowing you to do things like:
and more.
This is my first RFC, please forgive any missteps I make in the process.
Partially completes #1995.