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.

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

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

Merged
merged 1 commit into from
Nov 9, 2024

Conversation

compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented Oct 30, 2024

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
@mattheww
Copy link
Contributor

mattheww commented Oct 30, 2024

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

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

@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
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
@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Nov 9, 2024
@rfcbot
Copy link

rfcbot commented Nov 9, 2024

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@compiler-errors
Copy link
Member Author

@bors r=chenyukang

@bors
Copy link
Contributor

bors commented Nov 9, 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 9, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 9, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#132341 (Reject raw lifetime followed by `'`, like regular lifetimes do)
 - rust-lang#132363 (Enforce that raw lifetimes must be valid raw identifiers)
 - rust-lang#132744 (add regression test for rust-lang#90781)
 - rust-lang#132754 (Simplify the internal API for declaring command-line options)
 - rust-lang#132772 (use `download-rustc="if-unchanged"` as a global default)
 - rust-lang#132774 (Use lld with non-LLVM backends)
 - rust-lang#132799 (Make `Ty::primitive_symbol` recognize `str`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 9b50092 into rust-lang:master Nov 9, 2024
6 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Nov 9, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 9, 2024
Rollup merge of rust-lang#132341 - compiler-errors:raw-lt-prefix-id, 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.
mati865 pushed a commit to mati865/rust that referenced this pull request Nov 12, 2024
…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.
mati865 pushed a commit to mati865/rust that referenced this pull request Nov 12, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#132341 (Reject raw lifetime followed by `'`, like regular lifetimes do)
 - rust-lang#132363 (Enforce that raw lifetimes must be valid raw identifiers)
 - rust-lang#132744 (add regression test for rust-lang#90781)
 - rust-lang#132754 (Simplify the internal API for declaring command-line options)
 - rust-lang#132772 (use `download-rustc="if-unchanged"` as a global default)
 - rust-lang#132774 (Use lld with non-LLVM backends)
 - rust-lang#132799 (Make `Ty::primitive_symbol` recognize `str`)

r? `@ghost`
`@rustbot` modify labels: rollup
@cuviper cuviper mentioned this pull request Nov 15, 2024
@cuviper cuviper modified the milestones: 1.84.0, 1.83.0 Nov 15, 2024
@cuviper cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Nov 15, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 15, 2024
[beta] backports

- Use completion item indices instead of property matching rust-lang#132987, rust-lang/rust-analyzer#18503
- Reject raw lifetime followed by `'`, like regular lifetimes do rust-lang#132341
- Only disable cache if predicate has opaques within it rust-lang#132625
- rustdoc-search: case-sensitive only when capitals are used rust-lang#133043

r? cuviper
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 16, 2024
[beta] backports

- Use completion item indices instead of property matching rust-lang#132987, rust-lang/rust-analyzer#18503
- Reject raw lifetime followed by `'`, like regular lifetimes do rust-lang#132341
- Only disable cache if predicate has opaques within it rust-lang#132625
- rustdoc-search: case-sensitive only when capitals are used rust-lang#133043
- (ci) Update macOS Xcode to 15 rust-lang#131570

r? cuviper
@compiler-errors compiler-errors deleted the raw-lt-prefix-id branch November 20, 2024 04:27
@mattheww
Copy link
Contributor

I think this caused #133479

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted 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. finished-final-comment-period The final comment period is finished for this PR / Issue. I-lang-easy-decision Issue: The decision needed by the team is conjectured to be easy. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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. to-announce Announce this issue on triage meeting
Projects
None yet
Development

Successfully merging this pull request may close these issues.