Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Reserve prefixed identifiers and literals (RFC 3101) #85359

Merged
merged 14 commits into from
Jun 27, 2021
Merged

Reserve prefixed identifiers and literals (RFC 3101) #85359

merged 14 commits into from
Jun 27, 2021

Conversation

lrh2000
Copy link
Contributor

@lrh2000 lrh2000 commented May 16, 2021 •
edited by m-ou-se
Loading

This PR denies any identifiers immediately followed by one of three tokens ", ' or #, which is stricter than the requirements of RFC 3101 but may be necessary according to the discussion at Zulip.

The tracking issue #84599 says we'll add a feature gate named reserved_prefixes, but I don't think I can do this because it is impossible for the lexer to know whether a feature is enabled or not. I guess determining the behavior by the edition information should be enough.

Fixes #84599

@rust-highfive
Copy link
Collaborator

r? @jackh726

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 16, 2021
@lrh2000 lrh2000 force-pushed the reserved-prefixes branch from 582d02e to 4424c8c Compare May 16, 2021 03:45
@petrochenkov
Copy link
Contributor

cc #84037 (one more in-progress implementation) @Julian-Wollersberger

@petrochenkov
Copy link
Contributor

Could you assign this to me for the final implementation review, once it's decided who will implement this and what are the rules regarding raw strings etc.

@steffahn
Copy link
Member

Added some more comments on zulip (archive).

@lrh2000
Copy link
Contributor Author

lrh2000 commented May 16, 2021

cc #84037 (one more in-progress implementation) @Julian-Wollersberger

I guess it mainly focuses on implements the k# proposal now?

Could you assign this to me for the final implementation review, once it's decided who will implement this and what are the rules regarding raw strings etc.

OK, I'll do this.

Added some more comments on zulip (archive).

Thanks for your detailed feedback!

@jackh726
Copy link
Member

r? @petrochenkov

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 16, 2021
@petrochenkov petrochenkov mentioned this pull request May 17, 2021
Closed
@Julian-Wollersberger
Copy link
Contributor

I guess it mainly focuses on implements the k# proposal now?

Yes, and #84037 touches almost the same code. But since the k#foo RFC isn't accepted yet, it'll take some time. So I'd suggest that you (@lrh2000) go forward with this PR, and I'll later rebase or close the other one. (I don't have a strong opinion there.) Sounds good?

@lrh2000
Copy link
Contributor Author

lrh2000 commented May 22, 2021 •
edited
Loading

Yes, and #84037 touches almost the same code. But since the k#foo RFC isn't accepted yet, it'll take some time. So I'd suggest that you (@lrh2000) go forward with this PR, and I'll later rebase or close the other one. (I don't have a strong opinion there.) Sounds good?

@Julian-Wollersberger Yes, it sounds good. Sorry for the reply delay since I'm really busy in the past several days.

I don't think we have reached an agreement on the implementation details now (see discussions at #84978), so this PR may also take some time and the code may be completely rewritten.

@jack-champagne
Copy link
Contributor

I think you @'ed the wrong jack @lrh2000

@lrh2000
Copy link
Contributor Author

lrh2000 commented May 22, 2021

I think you @'ed the wrong jack @lrh2000

Oops, what a big mistake! I've fixed this and really sorry to bother you.

@crlf0710 crlf0710 added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 11, 2021
m-ou-se
m-ou-se reviewed Jun 14, 2021
View reviewed changes
compiler/rustc_parse/src/lexer/mod.rs Outdated Show resolved Hide resolved
compiler/rustc_parse/src/lexer/mod.rs Outdated Show resolved Hide resolved
@m-ou-se
Copy link
Member

m-ou-se commented Jun 14, 2021

Hope you don't mind, but I pushed the changes i suggested above, and added a migration lint for cargo fix --edition together with a test for that.

@m-ou-se m-ou-se added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Jun 14, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
nikomatsakis
nikomatsakis requested changes Jun 14, 2021
View reviewed changes
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. I left some nits.

compiler/rustc_lexer/src/lib.rs Outdated Show resolved Hide resolved
compiler/rustc_parse/src/lexer/mod.rs Outdated Show resolved Hide resolved
compiler/rustc_parse/src/lexer/mod.rs Outdated Show resolved Hide resolved
compiler/rustc_lexer/src/lib.rs Outdated Show resolved Hide resolved
compiler/rustc_lexer/src/lib.rs Outdated Show resolved Hide resolved
compiler/rustc_parse/src/lexer/mod.rs Outdated Show resolved Hide resolved
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@m-ou-se m-ou-se force-pushed the reserved-prefixes branch from dbf85fa to 8a7b3ae Compare June 14, 2021 14:20
@lrh2000 lrh2000 force-pushed the reserved-prefixes branch from 4e70c36 to 2bcd663 Compare June 26, 2021 15:13
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@petrochenkov
Copy link
Contributor

r? @nikomatsakis @bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented Jun 27, 2021

📌 Commit f6dd137 has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 27, 2021
@bors
Copy link
Contributor

bors commented Jun 27, 2021

⌛ Testing commit f6dd137 with merge e8cb1a4...

@bors
Copy link
Contributor

bors commented Jun 27, 2021

☀️ Test successful - checks-actions
Approved by: nikomatsakis
Pushing e8cb1a4 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 27, 2021
@bors bors merged commit e8cb1a4 into rust-lang:master Jun 27, 2021
@rustbot rustbot added this to the 1.55.0 milestone Jun 27, 2021
This was referenced Jun 27, 2021
Merged
Merged
@petrochenkov petrochenkov mentioned this pull request Jun 29, 2021
Merged
@lnicola lnicola mentioned this pull request Jul 10, 2021
Closed
@asdf8dfafjk
Copy link

asdf8dfafjk commented Jul 26, 2021 •
edited
Loading

This may be known to most people here but Scala allows people to make their own prefixes, say sql"" without the use of any compiler plugins. Here are some more details https://www.scala-lang.org/api/2.12.2/scala/StringContext.html

I'd love to see that being supported by compiler and a very small number of useful individual prefixes in the std-lib.

@m-ou-se m-ou-se mentioned this pull request Sep 3, 2021
Closed
@est31 est31 mentioned this pull request Apr 24, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Julian-Wollersberger Julian-Wollersberger Julian-Wollersberger left review comments

@m-ou-se m-ou-se m-ou-se left review comments

@nikomatsakis nikomatsakis nikomatsakis approved these changes

@matklad matklad matklad approved these changes

Assignees

@nikomatsakis nikomatsakis

Labels
A-edition-2021 Area: The 2021 edition A-parser Area: The parsing of Rust source code to an AST merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.55.0
Development

Successfully merging this pull request may close these issues.

Implement RFC: Reserved prefixes in the 2021 edition