-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
Support importing path-segment keyword with renaming #146972
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: main
Are you sure you want to change the base?
Conversation
|
Failed to set assignee to
|
The previous wording for this restriction was pretty confusing to me. I don't remember what I was thinking when I wrote it, and I can't find any historical explanation either. `use` paths can use `$crate` as long as they have more than one segment (`use $crate::foo` is obviously OK). I have rewritten this to make it clear it is specifically about `use $crate`. One could say that restriction is already covered by the previous point that says `use crate;` requires an `as`, but for some reason `use $crate as foo` doesn't work either. So I have left this as a separate rule for now. cc rust-lang/rust#146972 (comment) for context.
ce2578f to
49c425d
Compare
use $crate::{self} like use $crate
49c425d to
d0d3a9d
Compare
This comment has been minimized.
This comment has been minimized.
d0d3a9d to
c8526a5
Compare
This comment has been minimized.
This comment has been minimized.
c8526a5 to
db9bb42
Compare
This comment has been minimized.
This comment has been minimized.
df75a52 to
96820fa
Compare
This comment has been minimized.
This comment has been minimized.
96820fa to
1473c4c
Compare
This comment has been minimized.
This comment has been minimized.
1473c4c to
e0d5fa0
Compare
This comment has been minimized.
This comment has been minimized.
e0d5fa0 to
4c38159
Compare
This comment has been minimized.
This comment has been minimized.
4c38159 to
aca5e4e
Compare
This comment has been minimized.
This comment has been minimized.
aca5e4e to
30a19ad
Compare
|
Thanks to @mu001999 for putting this forward and to @petrochenkov for supporting this. This makes sense to me. I propose that we do it, and I'll then file a concern we discussed in the meeting (thanks @Nadrieril) about documentation. @rfcbot fcp merge lang |
|
Team member @traviscross 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. |
|
@rfcbot concern documentation The linked Reference PR (rust-lang/reference#2010) is a minor clarification. Let's please go ahead and also make a Reference PR documenting the rules described in this stabilization report. |
|
@rfcbot reviewed |
|
@rfcbot concern consistent-axioms I am a bit confused by why we would make errors in some of these cases and also the overall philosophical approach. In particular I expect as an "axiom" that, for all use $path::{self};
use $path::self;...but the PR description suggests that where My rule of thumb is that the language should accept things that have only one obvious meaning but, if they seem "redundant" or may suggest a broken mental model for users, issue a lint. Thinking over my model for how this works, I think of it like this...
This implies that e.g. |
|
@nikomatsakis We could certainly extend the language to make it a part of the path, but then I'd rather support |
|
☔ The latest upstream changes made this pull request unmergeable. Please resolve the merge conflicts. |
|
☔ The latest upstream changes (presumably #150682) made this pull request unmergeable. Please resolve the merge conflicts. |
c8f12de to
c679328
Compare
This comment has been minimized.
This comment has been minimized.
|
☔ The latest upstream changes made this pull request unmergeable. Please resolve the merge conflicts. |
|
☔ The latest upstream changes (presumably #150729) made this pull request unmergeable. Please resolve the merge conflicts. |
c679328 to
6439a5a
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
Hi @petrochenkov ... We discussed this in the lang-team meeting. Some notes from our discussion:
I see. This makes sense from that point-of-view, but I think the meeting consensus was that most users wouldn't see it that way, but rather that a syntax like (We did briefly consider something like the keyword you suggested or
We discussed this. Meeting consensus is that this is an orthogonal extension -- i.e., it's "consistent" to say that "if you can put it in braces, it should behave like other identifiers" and also say "...and you cannot use We felt the stronger case for allowing But after more discussion, we realized that supporting "backward relative operations" like super/crate adds a key bit of complexity when it comes to The meeting consensus was that this is definitely orthogonal (i.e., we don't have to support
Yes, we all agree this falls out from the axioms I laid out and out to work and be equivalent to In summary: Meeting consensus points
I realize we did not discuss something like |
|
I'd rather keep this PR doing what it does (i.e. bugfixing), modulo unbreaking the previously supported |
|
(Deleted my comment that was redundant with @nikomatsakis's summary.) |
|
Being explicit about something: we'd still love to see lints for cases like redundant use of (Those lints don't have to be in this PR, either.) |
Reference PR:
use $craterestriction reference#2010Description:
self/super/crate/$cratecan be imported with renaming nowuse self/super/crate/$crate as name;and alsouse super/crate/$crate::{self as name};use self/super/crate/$crate;and alsouse super/crate/$crate::{self};use ::self;,use ::{self};,use ::self as name;anduse ::{self as name};E0430andE0431are no longer emitted now[E0430]: self import can only appear once in an import listwas emitted before whenstd::fmt::{self, self}, but the emitted[E0252]: the name fmt is defined multiple timesis good enough[E0431]: self import can only appear in an import list with a non-empty prefixwas emiited before whenuse {self as name};, nowuse {self as name};is allowed; foruse {self};, it's denied because it lacks renaming and will emitimports need to be explicitly namedwith suggesting renaming.Fixes #29036
Fixes #35612
Fixes #37156
Fixes #146967
Fixes #149811
r? petrochenkov