Skip to content

Detect and provide suggestion for &raw EXPR #139392

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,17 @@ impl Path {
Path { segments: thin_vec![PathSegment::from_ident(ident)], span: ident.span, tokens: None }
}

pub fn is_ident(&self, name: Symbol) -> bool {
if let [segment] = self.segments.as_ref()
&& segment.args.is_none()
&& segment.ident.name == name
{
true
} else {
false
}
}

pub fn is_global(&self) -> bool {
self.segments.first().is_some_and(|segment| segment.ident.name == kw::PathRoot)
}
Expand Down
21 changes: 21 additions & 0 deletions compiler/rustc_parse/src/parser/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,8 @@ impl<'a> Parser<'a> {
// FIXME: translation requires list formatting (for `expect`)
let mut err = self.dcx().struct_span_err(self.token.span, msg_exp);

self.label_expected_raw_ref(&mut err);

// Look for usages of '=>' where '>=' was probably intended
if self.token == token::FatArrow
&& expected.iter().any(|tok| matches!(tok, TokenType::Operator | TokenType::Le))
Expand Down Expand Up @@ -750,6 +752,25 @@ impl<'a> Parser<'a> {
Err(err)
}

/// Adds a label when `&raw EXPR` was written instead of `&raw const EXPR`/`&raw mut EXPR`.
///
/// Given that not all parser diagnostics flow through `expected_one_of_not_found`, this
/// label may need added to other diagnostics emission paths as needed.
pub(super) fn label_expected_raw_ref(&mut self, err: &mut Diag<'_>) {
if self.prev_token.is_keyword(kw::Raw)
&& self.expected_token_types.contains(TokenType::KwMut)
&& self.expected_token_types.contains(TokenType::KwConst)
&& self.token.can_begin_expr()
{
err.span_suggestions(
self.prev_token.span.shrink_to_hi(),
"`&raw` must be followed by `const` or `mut` to be a raw reference expression",
[" const".to_string(), " mut".to_string()],
Applicability::MaybeIncorrect,
);
}
}

/// Checks if the current token or the previous token are misspelled keywords
/// and adds a helpful suggestion.
fn check_for_misspelled_kw(&self, err: &mut Diag<'_>, expected: &[TokenType]) {
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -829,6 +829,18 @@ impl<'a> Parser<'a> {
if let Some(lt) = lifetime {
self.error_remove_borrow_lifetime(span, lt.ident.span.until(expr.span));
}

// Add expected tokens if we parsed `&raw` as an expression.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 do we know anything about the parser grammar such that we could eagerly recover here (if self.may_recover(), of course), at least in some cases?

I don't think we ever expect an identifier token to follow another expr in valid rust, so if we see &raw IDENT, we could actually do recovery here rather than just failing later on in parsing.

// This will make sure we see "expected `const`, `mut`", and
// guides recovery in case we write `&raw expr`.
if borrow_kind == ast::BorrowKind::Ref
&& mutbl == ast::Mutability::Not
&& matches!(&expr.kind, ExprKind::Path(None, p) if p.is_ident(kw::Raw))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(minor) is_ident or this matches! needs to account for raw identifiers, so we can exclude r#raw (raw raw) here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a parsed ident, not an ident token, so we don't have a way to distinguish r#raw here, since ident is just a span and a symbol.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm 🤔 why is this being checked on the parsed expr and not as a lookahead after parsing the borrow modifiers? I guess kinda related to the recovery comment above, but would trivially allow checking for non-raw ident

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well we do want to avoid putting the expectation down for something like &raw.field

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yea

{
self.expected_token_types.insert(TokenType::KwMut);
self.expected_token_types.insert(TokenType::KwConst);
}

Ok((span, ExprKind::AddrOf(borrow_kind, mutbl, expr)))
}

Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_parse/src/parser/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,11 @@ impl<'a> Parser<'a> {
let prev = self.prev_token.span;
let sp = self.token.span;
let mut e = self.dcx().struct_span_err(sp, msg);
let do_not_suggest_help = self.token.is_keyword(kw::In) || self.token == token::Colon;
self.label_expected_raw_ref(&mut e);

let do_not_suggest_help = self.token.is_keyword(kw::In)
|| self.token == token::Colon
|| self.prev_token.is_keyword(kw::Raw);

// Check to see if the user has written something like
//
Expand Down
31 changes: 31 additions & 0 deletions tests/ui/parser/recover/raw-no-const-mut.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
fn a() {
let x = &raw 1;
//~^ ERROR expected one of
}

fn b() {
[&raw const 1, &raw 2]
//~^ ERROR expected one of
//~| ERROR cannot find value `raw` in this scope
//~| ERROR cannot take address of a temporary
}

fn c() {
if x == &raw z {}
//~^ ERROR expected `{`
}

fn d() {
f(&raw 2);
//~^ ERROR expected one of
//~| ERROR cannot find value `raw` in this scope
//~| ERROR cannot find function `f` in this scope
}

fn e() {
let x;
x = &raw 1;
//~^ ERROR expected one of
}

fn main() {}
109 changes: 109 additions & 0 deletions tests/ui/parser/recover/raw-no-const-mut.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
error: expected one of `!`, `.`, `::`, `;`, `?`, `const`, `else`, `mut`, `{`, or an operator, found `1`
--> $DIR/raw-no-const-mut.rs:2:18
|
LL | let x = &raw 1;
| ^ expected one of 10 possible tokens
|
help: `&raw` must be followed by `const` or `mut` to be a raw reference expression
|
LL | let x = &raw const 1;
| +++++
LL | let x = &raw mut 1;
| +++

error: expected one of `!`, `,`, `.`, `::`, `?`, `]`, `const`, `mut`, `{`, or an operator, found `2`
--> $DIR/raw-no-const-mut.rs:7:25
|
LL | [&raw const 1, &raw 2]
| ^ expected one of 10 possible tokens
|
help: `&raw` must be followed by `const` or `mut` to be a raw reference expression
|
LL | [&raw const 1, &raw const 2]
| +++++
LL | [&raw const 1, &raw mut 2]
| +++
help: missing `,`
|
LL | [&raw const 1, &raw, 2]
| +

error: expected `{`, found `z`
--> $DIR/raw-no-const-mut.rs:14:18
|
LL | if x == &raw z {}
| ^ expected `{`
|
note: the `if` expression is missing a block after this condition
--> $DIR/raw-no-const-mut.rs:14:8
|
LL | if x == &raw z {}
| ^^^^^^^^^
help: `&raw` must be followed by `const` or `mut` to be a raw reference expression
|
LL | if x == &raw const z {}
| +++++
LL | if x == &raw mut z {}
| +++

error: expected one of `!`, `)`, `,`, `.`, `::`, `?`, `const`, `mut`, `{`, or an operator, found `2`
--> $DIR/raw-no-const-mut.rs:19:12
|
LL | f(&raw 2);
| ^ expected one of 10 possible tokens
|
help: `&raw` must be followed by `const` or `mut` to be a raw reference expression
|
LL | f(&raw const 2);
| +++++
LL | f(&raw mut 2);
| +++
help: missing `,`
|
LL | f(&raw, 2);
| +

error: expected one of `!`, `.`, `::`, `;`, `?`, `const`, `mut`, `{`, `}`, or an operator, found `1`
--> $DIR/raw-no-const-mut.rs:27:14
|
LL | x = &raw 1;
| ^ expected one of 10 possible tokens
|
help: `&raw` must be followed by `const` or `mut` to be a raw reference expression
|
LL | x = &raw const 1;
| +++++
LL | x = &raw mut 1;
| +++

error[E0425]: cannot find value `raw` in this scope
--> $DIR/raw-no-const-mut.rs:7:21
|
LL | [&raw const 1, &raw 2]
| ^^^ not found in this scope

error[E0425]: cannot find value `raw` in this scope
--> $DIR/raw-no-const-mut.rs:19:8
|
LL | f(&raw 2);
| ^^^ not found in this scope

error[E0745]: cannot take address of a temporary
--> $DIR/raw-no-const-mut.rs:7:17
|
LL | [&raw const 1, &raw 2]
| ^ temporary value

error[E0425]: cannot find function `f` in this scope
--> $DIR/raw-no-const-mut.rs:19:5
|
LL | fn a() {
| ------ similarly named function `a` defined here
...
LL | f(&raw 2);
| ^ help: a function with a similar name exists: `a`

error: aborting due to 9 previous errors

Some errors have detailed explanations: E0425, E0745.
For more information about an error, try `rustc --explain E0425`.
Loading