Don't recover &raw EXPR as a missing comma

[qaijuang]

The parser already has a targeted suggestion for &raw EXPR, added in #139392, telling the user that &raw must be followed by const or mut.

In comma-separated expression lists, the generic missing-comma recovery would then treat the expression after raw as the next list element. That produced follow-up diagnostics like:

• help: missing ','
• cannot find value raw in this scope
• an arity error for calls such as takes_raw_ptr(&raw x)

This PR avoids that comma recovery when we are already in the &raw missing-const/mut diagnostic path. For function calls, it preserves the call expression with a single error argument, so useful later diagnostics such as cannot find function f are still emitted.

Fixes #157015.

[rustbot]

The parser was modified, potentially altering the grammar of (stable) Rust
which would be a breaking change.

cc @fmease

[rustbot]

r? @jieyouxu

rustbot has assigned @jieyouxu.
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

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 73 candidates
• Random selection from 19 candidates

[jieyouxu]

(Focusing on debuginfo/compiletest/rustfmt reviews this weekend)
@rustbot reroll

[Kivooeo]

Some nits

View changes since this review

[Kivooeo]

You'd want to factor out this check into separate method like is_addr_of_missing_const_mut(&self) -> bool, or came up with better naming

[qaijuang]

I had it in mind initially but needed validation like this.

given it'll be a parser/diagnostics method, name like is_expected_raw_ref_mut(&self) -> bool reads nicer with surrounding methods e.g the existing label_expected_raw_ref(&mut self, err: &mut Diag<'_>)

[Kivooeo]

Sounds good

[Kivooeo]

Can we also add a multi arguments test, like takes_raw_ptr(&raw x, x)

[qaijuang]

Good catch! 🤦🏾‍♂️

[Kivooeo]

hm, why this loop was introduced?

[qaijuang]

hm, why this loop was introduced?

it's basically to preserve the argument count for multi arguments

[Kivooeo]

how was the multi arguments diagnostics looked like without this last commit with loop

is that was buggy or something, need this understand what this is fixing

[qaijuang]

how was the multi arguments diagnostics looked like without this last commit with loop

is that was buggy or something, need this understand what this is fixing

it was buggy in the sense that it failed the multi args test

[Kivooeo]

i need to think more about this, so review might take some time

[Kivooeo]

ok, here is what i think. let's add this helper

fn recover_raw_ref_call_args(&mut self, guar: ErrorGuaranteed) -> ThinVec<Box<Expr>> {
    let err_span = self.prev_token.span.to(self.token.span);
    let mut args = thin_vec![self.mk_expr_err(err_span, guar)];
    while self.token != token::Eof && self.token != token::CloseParen {
        if self.eat(exp!(Comma)) {
            if self.token != token::Eof && self.token != token::CloseParen {
                args.push(self.mk_expr_err(self.prev_token.span.shrink_to_hi(), guar));
            }
        } else {
            self.parse_token_tree();
        }
    }
    let _ = self.eat(exp!(CloseParen));
    args
}

and then call site's become this

Err(err) if self.is_expected_raw_ref_mut() => {
    let guar = err.emit();
    let args = self.recover_raw_ref_call_args(guar);
    return self.mk_expr(lo.to(self.prev_token.span), self.mk_call(fun, args));
}

can you try and verify it's work as intended

[qaijuang]

ok, here is what i think. let's add this helper

fn recover_raw_ref_call_args(&mut self, guar: ErrorGuaranteed) -> ThinVec<Box<Expr>> {
    let err_span = self.prev_token.span.to(self.token.span);
    let mut args = thin_vec![self.mk_expr_err(err_span, guar)];
    while self.token != token::Eof && self.token != token::CloseParen {
        if self.eat(exp!(Comma)) {
            if self.token != token::Eof && self.token != token::CloseParen {
                args.push(self.mk_expr_err(self.prev_token.span.shrink_to_hi(), guar));
            }
        } else {
            self.parse_token_tree();
        }
    }
    let _ = self.eat(exp!(CloseParen));
    args
}

and then call site's become this

Err(err) if self.is_expected_raw_ref_mut() => {
    let guar = err.emit();
    let args = self.recover_raw_ref_call_args(guar);
    return self.mk_expr(lo.to(self.prev_token.span), self.mk_call(fun, args));
}

can you try and verify it's work as intended

Yes, it was part of my initial thought but I couldn’t think of its usefulness outside this issue?

[Kivooeo]

its just keep code clean i guess

[Kivooeo]

@bors r+ rollup

[rust-bors]

📌 Commit 68381b1 has been approved by Kivooeo

It is now in the queue for this repository.