Repository navigation
Improve suggestion for lifetime preceding type in generics - #163454
Merged
rust-bors[bot] merged 2 commits intoOct 3, 2026
Merged
rust-bors[bot] merged 2 commits into
rust-bors[bot] merged 2 commits into
Conversation
Collaborator
|
The parser was modified, potentially altering the grammar of (stable) Rust cc @fmease |
rustbot
added
S-waiting-on-review
Collaborator
|
Thanks for the pull request, and welcome! The Rust Project has assigned @JonathanBrouwer (or someone else) to review your changes, you should hear from them (or someone else) within the next two weeks. Please see the contribution instructions and our LLM policy for more information. Why was this reviewer chosen?The reviewer was selected based on:
|
tehforsch
commented
Sep 28, 2026
Comment on lines
+548
to
+557
| pub(super) fn parse_ref_ty_no_leading_ampersand(&mut self) -> (Mutability, Option<Box<Ty>>) { | ||
| let mutbl = self.parse_mutability(); | ||
| match self.parse_ty_no_plus() { | ||
| Ok(ty) => (mutbl, Some(ty)), | ||
| Err(err) => { | ||
| err.cancel(); | ||
| (mutbl, None) | ||
| } | ||
| } | ||
| } |
Contributor
Author
There was a problem hiding this comment.
I wasn't sure whether adding this helper is really worth it, especially since naming is hard
Member
There was a problem hiding this comment.
I think it's fine, there are plenty of helpers like this in the parser :)
tehforsch
commented
Sep 28, 2026
Comment on lines
+2298
to
+2300
| let snapshot = self.create_snapshot_for_diagnostic(); | ||
| let (mutbl, ty) = self.parse_ref_ty_no_leading_ampersand(); | ||
| self.restore_snapshot(snapshot); |
Contributor
Author
There was a problem hiding this comment.
I wasn't sure if maybe just doing
let mutbl = self.parse_mutability();
let is_ty = self.check_type();is enough, since (other than maybe_recover_ref_ty_no_leading_ampersand) we don't actually need the parsed type
tehforsch
changed the title
tehforsch
changed the title
JonathanBrouwer
approved these changes
Oct 3, 2026
Contributor
rust-bors
Bot
added
S-waiting-on-bors
rust-bors Bot
pushed a commit
that referenced
this pull request
Oct 3, 2026
…uwer Rollup of 8 pull requests Successful merges: - #163645 (add `ignore-compare-mode-next-solver` to all next-solver tests) - #163663 (reflection: fix ICE when TypeId::info called on ADT) - #163454 (Improve suggestion for lifetime preceding type in generics) - #163561 (tests: accept noundef in a couple more places) - #163633 (core: io: clean `unused_variables` lint under `no_io_statics`) - #163681 (Make the tracking issue template better suited for experiments) - #163685 (Avoid long iterator chain in all_rust_features) - #163690 (refactor: migrate to LLVM-C's `LLVMGetOrInsertFunction`) Failed merges: - #161491 (Rip out old solver coherence)
rust-bors Bot
pushed a commit
that referenced
this pull request
Oct 3, 2026
Rollup merge of #163454 - Tehforsch:fix-lifetime-preceding-type-in-generics, r=JonathanBrouwer Improve suggestion for lifetime preceding type in generics Closes #59325 Specifically, this fixes cases like the 2nd example in the issue, where a lifetime precedes a type in generic args: `Foo<'static T1>` and suggests either adding a reference, i.e. `Foo<&'static T1>` or a comma, i.e. `Foo<'static, T1>`. In a case like `Foo<'static mut T1>`, only `Foo<&'static mut T1>` is suggested.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #59325
Specifically, this fixes cases like the 2nd example in the issue, where a lifetime precedes a type in generic args:
Foo<'static T1>and suggests either adding a reference, i.e.Foo<&'static T1>or a comma, i.e.Foo<'static, T1>.In a case like
Foo<'static mut T1>, onlyFoo<&'static mut T1>is suggested.I initially wanted to do this similar to how the same situation is handled outside of generics (see
maybe_recover_ref_ty_no_leading_ampersand) where we recover something likename: 'a strasname: &'a str.However, in generics, both adding a
&and adding a,might be appropriate, so recovering by guessing either one of them might cause more, unrelated errors later. I tried constructing aTyKind::Err(_)and continuing to parse the rest of the generic arguments, but again I'd run into errors later during name resolution.Also finally, technically it's not super useful to suggest adding a comma in a case like
Foo<A, 'a B>, since, after applying the suggestion we would haveFoo<A, 'a, B>which isn't a valid type.However I am guessing (from the grammar) that this is valid in the AST and is only rejected by some stage after parsing, so I left the suggestion in.