Repository navigation
Recover missing turbofish for lifetime arguments in expression position - #162749
Open
raushan728 wants to merge 2 commits into
Open
raushan728 wants to merge 2 commits into
raushan728 wants to merge 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
|
r? @chenyukang rustbot has assigned @chenyukang. Use Why was this reviewer chosen?The reviewer was selected based on:
|
estebank
requested changes
Sep 14, 2026
Contributor
There was a problem hiding this comment.
I need to do a more in-depth review, as I have some mild concerns about increasing the size of the parser, but the results look reasonable so far. I'll look at this again later this week if the assigned reviewer doesn't manage to get the time before then.
rustbot
added
S-waiting-on-author
Collaborator
|
Reminder, once the PR becomes ready for a review, use |
raushan728
force-pushed
the
issues/162656
branch
from
September 15, 2026 14:20
acf53ca to
ce2a12a
Compare
This comment has been minimized.
This comment has been minimized.
raushan728
commented
Sep 15, 2026
Contributor
Author
|
@rustbot ready |
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
raushan728
force-pushed
the
issues/162656
branch
from
September 16, 2026 11:05
ce2a12a to
2e5a236
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Member
|
r? @estebank since you already take review :) |
raushan728
changed the title
turbofish for lifetime arguments in expression position
raushan728
force-pushed
the
issues/162656
branch
from
September 23, 2026 13:59
2e5a236 to
2b3bcf9
Compare
This comment has been minimized.
This comment has been minimized.
raushan728
force-pushed
the
issues/162656
branch
from
September 25, 2026 13:21
2b3bcf9 to
6836140
Compare
This comment has been minimized.
This comment has been minimized.
raushan728
force-pushed
the
issues/162656
branch
from
October 2, 2026 14:12
6836140 to
976f9ba
Compare
Contributor
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
rustbot
added
the
S-waiting-on-perf
This comment has been minimized.
This comment has been minimized.
rust-bors Bot
pushed a commit
that referenced
this pull request
Oct 8, 2026
Recover missing `turbofish` for `lifetime` arguments in expression position
Contributor
|
It makes me nervous how much code this special case contains and the complexity of the recovery. (This kind of recovery code makes me groan when I'm making changes to the parser.) Other recovery paths in this function are moved to separate functions, the same should be done for this. For readability and maybe also for performance. (The initial check could still be done inline.) Is How will cases like these be handled? Will the code after the Overall I'm not convinced this is worth the complexity and risk in this tricky and performance-sensitive part of the parser :( |
nnethercote
added
S-waiting-on-author
Contributor
This comment has been minimized.
This comment has been minimized.
fmease
reviewed
Oct 8, 2026
|
|
||
| lhs = match op.node { | ||
| AssocOp::Binary(ast_op) => { | ||
| if ast_op == ast::BinOpKind::Lt |
Member
There was a problem hiding this comment.
I haven't read this code but if you end up keeping some form of it (cc #162749 (comment)) please move it out of line into a new pub(super) fn recover_from_* function in compiler/rustc_parse/src/parser/expr/diagnostics.rs.
CC my ongoing effort to make the parser maintainable again: #162591.
Contributor
Author
There was a problem hiding this comment.
expr.rs is no longer touched at all. The only inline code is a condition in parse_path_segment (token == Lt is checked first).
Collaborator
|
Finished benchmarking commit (e67d7c3): comparison URL. Overall result: no relevant changes - no action neededBenchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up. @rustbot label: -S-waiting-on-perf -perf-regression Instruction countThis perf run didn't have relevant results for this metric. Max RSS (memory usage)Results (secondary 1.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -2.3%, secondary 4.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 487.56s -> 486.365s (-0.25%) |
rustbot
removed
the
S-waiting-on-perf
raushan728
force-pushed
the
issues/162656
branch
from
October 9, 2026 10:51
2a84483 to
a122d5d
Compare
Collaborator
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
raushan728
commented
Oct 9, 2026
Contributor
Author
There was a problem hiding this comment.
Is
path.spancorrect after recovery?
It is now. The recovery moved from parse_expr_assoc_rest into parse_path_segment for PathStyle::Expr, so the generic args go through the normal parsing path and the path span covers the <'a> part.
How will cases like these be handled?
Struct<'a>::new() Struct<'a> { x }.method() Struct<'a> { x }?
Each now produces only the single turbofish error, with no follow up syntax errors. All three are covered in test.
Contributor
Author
|
@rustbot ready |
rustbot
added
S-waiting-on-review
Collaborator
|
The job Click to see the possible cause of the failure (guessed by this bot) |
rustbot
removed
the
S-waiting-on-review
rustbot
added
the
S-waiting-on-review
This branch has not been deployed
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
8 participants
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.
View all comments
Fixes #162656.
Struct<'a> { .. }andf<'_>()were parsed as a<comparison whose RHS is a label, producing a cascade of unrelated errors.parse_path_segmentnow accepts<followed by a lifetime and>/,in expression paths, emits one "use::<...>" suggestion, and parses the args normally.Since the recovery is inside the path parser,path.spanand chained forms (::new(),.method(),?) work without AST fix-ups or newParserstate; it is guarded bymay_recover().cc @estebank