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

Reject raw lifetime followed by ', like regular lifetimes do #132341

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Reject raw lifetime followed by ', like regular lifetimes do #132341

wants to merge 1 commit into from
+38 −1

Conversation

compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented Oct 30, 2024 •
edited
Loading

See comment. We want to reject cases like 'r#long'id, which currently gets interpreted as a raw lifetime ('r#long) followed by a lifetime ('id). This could have alternative lexes, such as an overlong char literal ('r#long') followed by an identifier (id). To avoid committing to this in any case, let's reject the whole thing.

@mattheww, is this what you were looking for in rust-lang/reference#1603 (comment)? I'd say ignore the details about the specific error message (the fact that this gets reinterpreted as a char literal is 🤷), just that because this causes a lexer error we're effectively saving syntactical space like you wanted.

@compiler-errors compiler-errors marked this pull request as ready for review October 30, 2024 01:13
@rustbot
Copy link
Collaborator

rustbot commented Oct 30, 2024

r? @chenyukang

rustbot has assigned @chenyukang.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 30, 2024
@compiler-errors compiler-errors added I-lang-nominated Nominated for discussion during a lang team meeting. I-lang-easy-decision Issue: The decision needed by the team is conjectured to be easy. labels Oct 30, 2024
@compiler-errors
Copy link
Member Author

Nominating as an easy decision because this is really new syntax and it saves space.

If we want to go through with this, I'd say we also beta nominate it because raw lifetimes are landing next edition, though I guess we could land them out of sync because I doubt anyone will begin relying on this obscure behavior of a somewhat obscure feature in 6 weeks.

@compiler-errors compiler-errors added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 30, 2024
@compiler-errors compiler-errors changed the title Reject raw lifetime followed by ' like lifetimes Reject raw lifetime followed by ', like regular lifetimes do Oct 30, 2024
@compiler-errors compiler-errors mentioned this pull request Oct 30, 2024
Open
@mattheww
Copy link
Contributor

mattheww commented Oct 30, 2024 •
edited
Loading

Yes, that's what I was thinking of.

I think #132341 (comment) has a typo, and should be saying

We want to reject cases like 'r#long'id

@joshtriplett joshtriplett added T-lang Relevant to the language team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 30, 2024
@joshtriplett
Copy link
Member

@rfcbot merge

@rfcbot
Copy link

rfcbot commented Oct 30, 2024 •
edited by traviscross
Loading

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

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.
See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Oct 30, 2024
@joshtriplett joshtriplett added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 30, 2024
@tmandry
Copy link
Member

tmandry commented Oct 30, 2024

@rfcbot reviewed

1 similar comment
@nikomatsakis
Copy link
Contributor

@rfcbot reviewed

@traviscross
Copy link
Contributor

@rfcbot reveiwed

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Oct 30, 2024
@rfcbot
Copy link

rfcbot commented Oct 30, 2024

🔔 This is now entering its final comment period, as per the review above. 🔔

@traviscross
Copy link
Contributor

traviscross commented Oct 30, 2024 •
edited
Loading

@rustbot labels -I-lang-nominated

We discussed this in triage today, and as above, this seemed a clear call to us.

@rustbot rustbot removed the I-lang-nominated Nominated for discussion during a lang team meeting. label Oct 30, 2024
@apiraino
Copy link
Contributor

Beta backport accepted as per compiler team on Zulip, after it gets merged. A backport PR will be authored by the release team at the end of the current development cycle.

@rustbot label +beta-accepted

@rustbot rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Oct 31, 2024
@compiler-errors compiler-errors added S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 31, 2024
@chenyukang
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Nov 1, 2024

📌 Commit 1990f15 has been approved by chenyukang

It is now in the queue for this repository.

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Nov 1, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 1, 2024
@matthiaskrgr
Rollup merge of rust-lang#132341 - compiler-errors:raw-lt-prefix-id, …
825acc2 
…r=chenyukang

Reject raw lifetime followed by `'`, like regular lifetimes do

See comment. We want to reject cases like `'r#long'id`, which currently gets interpreted as a raw lifetime (`'r#long`) followed by a lifetime (`'id`). This could have alternative lexes, such as an overlong char literal (`'r#long'`) followed by an identifier (`id`). To avoid committing to this in any case, let's reject the whole thing.

`@mattheww,` is this what you were looking for in rust-lang/reference#1603 (comment)? I'd say ignore the details about the specific error message (the fact that this gets reinterpreted as a char literal is 🤷), just that because this causes a lexer error we're effectively saving syntactical space like you wanted.
@matthiaskrgr matthiaskrgr mentioned this pull request Nov 1, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 1, 2024
@bors
Auto merge of rust-lang#132446 - matthiaskrgr:rollup-qm3yilw, r=matth…
e468296 
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#131829 (Remove support for `-Zprofile` (gcov-style coverage instrumentation))
 - rust-lang#132341 (Reject raw lifetime followed by `'`, like regular lifetimes do)
 - rust-lang#132369 (style-guide: Only use the new binop heuristic for assignments)
 - rust-lang#132383 (Implement suggestion for never type fallback lints)
 - rust-lang#132437 (coverage: Regression test for inlining into an uninstrumented crate)
 - rust-lang#132438 (Remove unncessary option for default rust-analyzer setting)

r? `@ghost`
`@rustbot` modify labels: rollup
@compiler-errors
Copy link
Member Author

@chenyukang: Please wait until the end of an FCP to approve a PR. The status of this approval could still technically change in the next 10 days.

@compiler-errors
Copy link
Member Author

Given that it's already rolled up though, I guess this can just merge now.

@compiler-errors
Copy link
Member Author

@bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 1, 2024
@compiler-errors compiler-errors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@chenyukang chenyukang

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. beta-nominated Nominated for backporting to the compiler in the beta channel. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. I-lang-easy-decision Issue: The decision needed by the team is conjectured to be easy. S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.