Recover missing turbofish for lifetime arguments in expression position - #162749
raushan728 wants to merge 3 commits into
Conversation
|
The parser was modified, potentially altering the grammar of (stable) Rust cc @fmease |
|
r? @chenyukang rustbot has assigned @chenyukang. Use Why was this reviewer chosen?The reviewer was selected based on:
|
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.
|
Reminder, once the PR becomes ready for a review, use |
acf53ca to
ce2a12a
Compare
This comment has been minimized.
This comment has been minimized.
| let (op_span, ident) = self.expected_turbofish_context.take().expect( | ||
| "IS_RHS_OF_LT_AFTER_PATH is only ever set alongside expected_turbofish_context \ | ||
| in parse_expr_assoc_rest, so this must be Some here", |
There was a problem hiding this comment.
code handled three cases when building the turbofish suggestion a path aware span_suggestion, a fallback span_suggestion using just a bare lifetime span, and a final bare .help() with no span at all. In practice only the first one is ever reachable the restriction flag that gates this whole recovery branch (IS_RHS_OF_LT_AFTER_PATH) and the path context used to build the message are always set together, in the same code path in parse_expr_assoc_rest, and a Path is guaranteed to have at least one segment. So the other two branches could never actually fire with the current call graph.
I'd originally kept them as a defensive fallback in case some future change set the restriction flag from a different call site without also populating the context - but on reflection that's the wrong kind of defensiveness for a compiler internal it would silently degrade to a less informative message instead of surfacing the broken invariant. I replaced it with a single .expect() carrying an explicit message about the invariant, so if this assumption is ever violated by future changes, it fails loudly and immediately at the exact point of violation.
This also drops the now-unused lt_span parameter from parse_expr_labeled entirely, which was only ever feeding the dead fallback branch.
|
@rustbot ready |
This comment has been minimized.
This comment has been minimized.
ce2a12a to
2e5a236
Compare
|
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. |
Fixes #162656.
rustc already suggests
::<>when a type/const generic argument is missing its turbofish (foo<Bar>()), but not for lifetime arguments -Struct<'a> { .. }previously misfired through label/char-literal recovery, producing a confusing cascade of unrelated errors instead of one actionable turbofish suggestion.cc @estebank