fix ICE when parsing lifetime as function argument

[compiler-errors]

I don't really like this, but we basically need to emit an error instead of just delaying an bug, because there are too many places in the AST that aren't covered by my previous PRs...

cc: #93282 (comment)

[rust-highfive]

r? @jackh726

(rust-highfive has picked a reviewer for you, use r? to override)

[jackh726]

This doesn't quite seem right :)

I would have expected that the "recovery" here is just to fall back to generics with a lifetime.

[compiler-errors]

I would have expected that the "recovery" here is just to fall back to generics with a lifetime.

@jackh726, this fix is actually for when we parse lifetimes in the expression position. This happens when we have a lifetime token as the first token when parsing a loop expression.

So the deal is that we delayed a bug in the code, but none of the paths that I've previously added which do recovery for bad generics apply to something like:

fn main() {
  function('a,
        // ^^- here

[compiler-errors]

Not sure how to fix this other than emitting a sometimes-unhelpful error. This error existed in the past, before I added the delay-span-bug that I reverted here.

For context: #92876 (specifically this)

[compiler-errors]

Ok, this definitely got included from another PR of mine. I'll revert this!

[jackh726]

For the function arg case, I could see this as acceptable (given that you actually can use a labeled expr as an argument - or at least I would expect that to work). For the generics though, I'm a bit surprised we get to parse_labeled_expr in the first place. Maybe consts could have a labeled expr as a value (but I would expect those to have to be enclosed in {}). This can probably be conditional on if we're enclosed in () vs {}

[compiler-errors]

Well in the case we have an expr x < 'a, we need to parse that 'a as an expression (just the loop label), and then we parse the whole thing as a comparison operator. This is important for the recovery in the case we do have a bad turbofish. But in the case we don't have a bad turbofish, we need to emit an error.

We can't just raise a PResult::Err when we see a bare 'a in expr position, because we have no way of distinguishing that the error is one we can recover as turbofish generics.

It's just because the recovery is split over too many layers, since that < looks like an expression until much later, when we see >, and we try to recover the chained comparison operator error.

[compiler-errors]

I think that explanation of mine is a bit complicated. To put it in a different way:

We don't start recovering a bad turbofish until after we have parsed the bad label expression. If we continue to delay a bug in the label parsing code like we're doing now, we must be absolutely certain we're going to emit another bug later on all codepaths we took to get here, in order to prevent an ICE (sorry, you definitely know this already, just want to spell out my thought process clearly).

It turns out this doesn't always work out, since we cannot bubble up the information that we've parsed a naked lifetime label token through the various layers of parse_expr, since we just return an expression (and doing so would be a very heavy change). We can't bubble down the expectation that we might be currently parsing something that looks like a bad turbofish either, because we already parsed the < of the turbofish as a regular operator expression.

This example, which ICEs, might help demonstrate something that locally looks like a bad turbofish, but which parses (incorrectly) today, since we only delay a bug when we parse that bad lifetime 'a.

fn f(_a1: bool, _a2: bool) {
    let a = 1i32;
    let b = 1i32;
    f(a < 'a, b > (1))
}

This PR just adds emitting an error back to the label parsing code, which I supressed in previous PR. I'd rather us possibly emit two errors, where one is redundant, and emit zero and ICE on certain pathological cases.

[jackh726]

@bors r+ rollup

[bors]

📌 Commit 5be9e79 has been approved by jackh726