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

Enforce that raw lifetimes must be valid raw identifiers #132363

Merged
merged 1 commit into from
Nov 9, 2024
Merged

Enforce that raw lifetimes must be valid raw identifiers #132363

merged 1 commit into from
Nov 9, 2024

Conversation

compiler-errors
Copy link
Member

Make sure that the identifier part of a raw lifetime is a valid raw identifier. This precludes 'r#_ and all module segment paths for now.

I don't believe this is compelling to support. This was raised by @ehuss in rust-lang/reference#1603 (comment) (well, specifically the 'r#_ case), but I don't see why we shouldn't just make it consistent with raw identifiers.

@rustbot
Copy link
Collaborator

rustbot commented Oct 30, 2024

r? @wesleywiser

rustbot has assigned @wesleywiser.
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 beta-nominated Nominated for backporting to the compiler in the beta channel. 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; this does not imply nomination and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 30, 2024
@compiler-errors
Copy link
Member Author

Nominating for T-lang once again, seems like an easy decision b/c it's just slighlty paring back the generality of a new feature stabilized in the next rust version. For that reason I'm also beta nominating it, just like ##132341.

@nikomatsakis
Copy link
Contributor

@rustbot labels +T-lang

@rustbot rustbot added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Oct 30, 2024
@nikomatsakis
Copy link
Contributor

nikomatsakis commented Oct 30, 2024 •
edited
Loading

@rfcbot fcp merge

Consistency across classes of identifiers seems correct.

@rfcbot
Copy link

rfcbot commented Oct 30, 2024 •
edited
Loading

Team member @nikomatsakis 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
@traviscross
Copy link
Contributor

@rfcbot reviewed

1 similar comment
@scottmcm
Copy link
Member

@rfcbot reviewed

@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. 🔔

@tmandry
Copy link
Member

tmandry commented Oct 30, 2024

@rfcbot reviewed

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

@rustbot labels -I-lang-nominated

We discussed this in triage today, and as above, we liked this. We could always decide to allow more later. Being consistent and conservative seems right for now.

wesleywiser
wesleywiser approved these changes Oct 30, 2024
View reviewed changes
Copy link
Member

@wesleywiser wesleywiser left a comment

Choose a reason for hiding this comment

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

r=me when FCP has appropriately completed 🙂

@compiler-errors compiler-errors added the S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. label Oct 31, 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.

@rustbot rustbot mentioned this pull request Nov 9, 2024
Closed
3 tasks
@compiler-errors
Copy link
Member Author

@bors r=wesleywiser

@bors
Copy link
Contributor

bors commented Nov 9, 2024

📌 Commit 9785c7c has been approved by wesleywiser

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
@matthiaskrgr matthiaskrgr mentioned this pull request Nov 9, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 9, 2024
@bors
Auto merge of rust-lang#132815 - matthiaskrgr:rollup-nti992u, r=matth…
4adafcf 
…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 910ac7e 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
@rust-timer
Unrolled build for rust-lang#132363
e909e53 
Rollup merge of rust-lang#132363 - compiler-errors:raw-lt-id-valid, r=wesleywiser

Enforce that raw lifetimes must be valid raw identifiers

Make sure that the identifier part of a raw lifetime is a valid raw identifier. This precludes `'r#_` and all module segment paths for now.

I don't believe this is compelling to support. This was raised by `@ehuss` in rust-lang/reference#1603 (comment) (well, specifically the `'r#_` case), but I don't see why we shouldn't just make it consistent with raw identifiers.
mati865 pushed a commit to mati865/rust that referenced this pull request Nov 12, 2024
@matthiaskrgr @mati865
Rollup merge of rust-lang#132363 - compiler-errors:raw-lt-id-valid, r…
3aa71d7 
…=wesleywiser

Enforce that raw lifetimes must be valid raw identifiers

Make sure that the identifier part of a raw lifetime is a valid raw identifier. This precludes `'r#_` and all module segment paths for now.

I don't believe this is compelling to support. This was raised by `@ehuss` in rust-lang/reference#1603 (comment) (well, specifically the `'r#_` case), but I don't see why we shouldn't just make it consistent with raw identifiers.
mati865 pushed a commit to mati865/rust that referenced this pull request Nov 12, 2024
@bors @mati865
Auto merge of rust-lang#132815 - matthiaskrgr:rollup-nti992u, r=matth…
2496d2b 
…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
ehuss added a commit to compiler-errors/reference that referenced this pull request Nov 14, 2024
@ehuss
Clarify raw lifetime cannot be certain keywords
912a6d5 
Update for rust-lang/rust#132363
@ehuss ehuss mentioned this pull request Nov 14, 2024
Merged
ehuss added a commit to compiler-errors/reference that referenced this pull request Nov 15, 2024
@ehuss
Clarify raw lifetime cannot be certain keywords
f04cd63 
Update for rust-lang/rust#132363
@cuviper
Copy link
Member

cuviper commented Nov 19, 2024

Should this get the T-compiler label back to get the backport on their agenda? Or should T-lang decide that too?

@cuviper cuviper added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Nov 19, 2024
@traviscross
Copy link
Contributor

Thanks. From the lang side, when we made the decision above, we had intended for this to happen if possible in the same release as the related feature, as with #132341, so it's safe to say that we're in favor of a beta backport if the compiler team agrees.

@compiler-errors
Copy link
Member Author

Ah, right, this wasn't tagged correctly for T-compiler triage.

@apiraino
Copy link
Contributor

Beta backport approved as per compiler team on Zulip. 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 Nov 21, 2024
@cuviper cuviper mentioned this pull request Nov 21, 2024
Merged
@cuviper cuviper modified the milestones: 1.84.0, 1.83.0 Nov 21, 2024
@cuviper cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Nov 21, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 22, 2024
@bors
Auto merge of rust-lang#133305 - cuviper:beta-next, r=cuviper
b35ad72 
[beta] backports

- Enforce that raw lifetimes must be valid raw identifiers rust-lang#132363
- Update cdb annotations for some debuginfo tests with cdb `10.0.26100.2161` rust-lang#133115

r? cuviper
mattheww added a commit to mattheww/lexeywan that referenced this pull request Dec 1, 2024
@mattheww
RawLifetimeOrLabel reprocessing: reject some keywords
4f2cf4b 
This mirrors the changes from
rust-lang/rust#132363
"Enforce that raw lifetimes must be valid raw identifiers"
which have been backported to appear in Rust 1.83
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wesleywiser wesleywiser wesleywiser approved these changes

Assignees

@wesleywiser wesleywiser

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; this does not imply nomination 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
Milestone
1.83.0
Development

Successfully merging this pull request may close these issues.