Skip to content

improve error message when global_asm! uses asm! operands #128305

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.

Sign up for GitHub

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
Aug 4, 2024
Merged

improve error message when global_asm! uses asm! operands #128305

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions compiler/rustc_builtin_macros/messages.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,9 @@ builtin_macros_format_use_positional = consider using a positional formatting ar

builtin_macros_global_asm_clobber_abi = `clobber_abi` cannot be used with `global_asm!`

builtin_macros_global_asm_unsupported_operand = the `{$symbol}` operand cannot be used with `global_asm!`
.label = the `{$symbol}` operand is not meaningful for global-scoped inline assembly, remove it
Comment on lines +202 to +203
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on https://doc.rust-lang.org/reference/inline-assembly.html#syntax, it seems like in/out/inout/etc are "direction specifiers", while "operand" includes the variable. So maybe "specifier" rather than "operand" here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the word "operand" is used for the whole thing, so all of x = inout(reg) x, is an operand. The word "specifier" does not occur on that page, "spec" only in the grammar.

The section describing these items is https://doc.rust-lang.org/reference/inline-assembly.html#operand-type. It says that "Several types of operands are supported" and then lists in, out etc. So I think using the word "operand" is the most helpful.

The error message points to the specifier specifically because it is easy to parse without emitting any unhelpful errors, but the operand as a whole is unsupported.


builtin_macros_global_asm_unsupported_option = the `{$symbol}` option cannot be used with `global_asm!`
.label = the `{$symbol}` option is not meaningful for global-scoped inline assembly
.suggestion = remove this option
Expand Down
39 changes: 31 additions & 8 deletions compiler/rustc_builtin_macros/src/asm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,29 @@ pub struct AsmArgs {
pub options_spans: Vec<Span>,
}

/// Used for better error messages when operand types are used that are not
/// supported by the current macro (e.g. `in` or `out` for `global_asm!`)
///
/// returns
///
/// - `Ok(true)` if the current token matches the keyword, and was expected
/// - `Ok(false)` if the current token does not match the keyword
/// - `Err(_)` if the current token matches the keyword, but was not expected
fn eat_operand_keyword<'a>(p: &mut Parser<'a>, symbol: Symbol, expect: bool) -> PResult<'a, bool> {
Comment on lines +32 to +40
Copy link
Contributor

Choose a reason for hiding this comment

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

Every time this gets called, expect is !is_global_asm. Maybe change expect to disallowed and reverse that logic, so you only need to pass is_global_asm?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

like the previous PR, this is in preparation of adding naked_asm!. That will pass an enum as the third argument to decide whether the symbol is expected. For now I think going with the concept of "expected" makes sense because that is the terminology that the parser uses.

if expect {
Ok(p.eat_keyword(symbol))
} else {
let span = p.token.span;
if p.eat_keyword_noexpect(symbol) {
// in gets printed as `r#in` otherwise
let symbol = if symbol == kw::In { "in" } else { symbol.as_str() };
Err(p.dcx().create_err(errors::GlobalAsmUnsupportedOperand { span, symbol }))
} else {
Ok(false)
}
}
}

fn parse_args<'a>(
ecx: &ExtCtxt<'a>,
sp: Span,
Expand Down Expand Up @@ -106,23 +129,23 @@ pub fn parse_asm_args<'a>(
};

let mut explicit_reg = false;
let op = if !is_global_asm && p.eat_keyword(kw::In) {
let op = if eat_operand_keyword(p, kw::In, !is_global_asm)? {
let reg = parse_reg(p, &mut explicit_reg)?;
if p.eat_keyword(kw::Underscore) {
let err = dcx.create_err(errors::AsmUnderscoreInput { span: p.token.span });
return Err(err);
}
let expr = p.parse_expr()?;
ast::InlineAsmOperand::In { reg, expr }
} else if !is_global_asm && p.eat_keyword(sym::out) {
} else if eat_operand_keyword(p, sym::out, !is_global_asm)? {
let reg = parse_reg(p, &mut explicit_reg)?;
let expr = if p.eat_keyword(kw::Underscore) { None } else { Some(p.parse_expr()?) };
ast::InlineAsmOperand::Out { reg, expr, late: false }
} else if !is_global_asm && p.eat_keyword(sym::lateout) {
} else if eat_operand_keyword(p, sym::lateout, !is_global_asm)? {
let reg = parse_reg(p, &mut explicit_reg)?;
let expr = if p.eat_keyword(kw::Underscore) { None } else { Some(p.parse_expr()?) };
ast::InlineAsmOperand::Out { reg, expr, late: true }
} else if !is_global_asm && p.eat_keyword(sym::inout) {
} else if eat_operand_keyword(p, sym::inout, !is_global_asm)? {
let reg = parse_reg(p, &mut explicit_reg)?;
if p.eat_keyword(kw::Underscore) {
let err = dcx.create_err(errors::AsmUnderscoreInput { span: p.token.span });
Expand All @@ -136,7 +159,7 @@ pub fn parse_asm_args<'a>(
} else {
ast::InlineAsmOperand::InOut { reg, expr, late: false }
}
} else if !is_global_asm && p.eat_keyword(sym::inlateout) {
} else if eat_operand_keyword(p, sym::inlateout, !is_global_asm)? {
let reg = parse_reg(p, &mut explicit_reg)?;
if p.eat_keyword(kw::Underscore) {
let err = dcx.create_err(errors::AsmUnderscoreInput { span: p.token.span });
Expand All @@ -150,6 +173,9 @@ pub fn parse_asm_args<'a>(
} else {
ast::InlineAsmOperand::InOut { reg, expr, late: true }
}
} else if eat_operand_keyword(p, sym::label, !is_global_asm)? {
let block = p.parse_block()?;
ast::InlineAsmOperand::Label { block }
} else if p.eat_keyword(kw::Const) {
let anon_const = p.parse_expr_anon_const()?;
ast::InlineAsmOperand::Const { anon_const }
Expand All @@ -165,9 +191,6 @@ pub fn parse_asm_args<'a>(
path: path.clone(),
};
ast::InlineAsmOperand::Sym { sym }
} else if !is_global_asm && p.eat_keyword(sym::label) {
let block = p.parse_block()?;
ast::InlineAsmOperand::Label { block }
} else if allow_templates {
let template = p.parse_expr()?;
// If it can't possibly expand to a string, provide diagnostics here to include other
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_builtin_macros/src/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -856,6 +856,15 @@ pub(crate) struct GlobalAsmUnsupportedOption {
pub(crate) full_span: Span,
}

#[derive(Diagnostic)]
#[diag(builtin_macros_global_asm_unsupported_operand)]
pub(crate) struct GlobalAsmUnsupportedOperand<'a> {
#[primary_span]
#[label]
pub(crate) span: Span,
pub(crate) symbol: &'a str,
}

#[derive(Diagnostic)]
#[diag(builtin_macros_test_runner_invalid)]
pub(crate) struct TestRunnerInvalid {
Expand Down
15 changes: 13 additions & 2 deletions tests/ui/asm/parse-error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,5 +146,16 @@ global_asm!(format!("{{{}}}", 0), const FOO);
//~^ ERROR asm template must be a string literal
global_asm!("{1}", format!("{{{}}}", 0), const FOO, const BAR);
//~^ ERROR asm template must be a string literal
global_asm!("{}", label {});
//~^ ERROR expected operand, options, or additional template string

global_asm!("{}", in(reg));
//~^ ERROR the `in` operand cannot be used with `global_asm!`
global_asm!("{}", out(reg));
//~^ ERROR the `out` operand cannot be used with `global_asm!`
global_asm!("{}", lateout(reg));
//~^ ERROR the `lateout` operand cannot be used with `global_asm!`
global_asm!("{}", inout(reg));
//~^ ERROR the `inout` operand cannot be used with `global_asm!`
global_asm!("{}", inlateout(reg));
//~^ ERROR the `inlateout` operand cannot be used with `global_asm!`
global_asm!("{}", label(reg));
//~^ ERROR the `label` operand cannot be used with `global_asm!`
40 changes: 35 additions & 5 deletions tests/ui/asm/parse-error.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,11 +380,41 @@ LL | global_asm!("{1}", format!("{{{}}}", 0), const FOO, const BAR);
|
= note: this error originates in the macro `format` (in Nightly builds, run with -Z macro-backtrace for more info)

error: expected operand, options, or additional template string
--> $DIR/parse-error.rs:149:19
error: the `in` operand cannot be used with `global_asm!`
--> $DIR/parse-error.rs:150:19
|
LL | global_asm!("{}", in(reg));
| ^^ the `in` operand is not meaningful for global-scoped inline assembly, remove it

error: the `out` operand cannot be used with `global_asm!`
--> $DIR/parse-error.rs:152:19
|
LL | global_asm!("{}", out(reg));
| ^^^ the `out` operand is not meaningful for global-scoped inline assembly, remove it

error: the `lateout` operand cannot be used with `global_asm!`
--> $DIR/parse-error.rs:154:19
|
LL | global_asm!("{}", lateout(reg));
| ^^^^^^^ the `lateout` operand is not meaningful for global-scoped inline assembly, remove it

error: the `inout` operand cannot be used with `global_asm!`
--> $DIR/parse-error.rs:156:19
|
LL | global_asm!("{}", inout(reg));
| ^^^^^ the `inout` operand is not meaningful for global-scoped inline assembly, remove it

error: the `inlateout` operand cannot be used with `global_asm!`
--> $DIR/parse-error.rs:158:19
|
LL | global_asm!("{}", inlateout(reg));
| ^^^^^^^^^ the `inlateout` operand is not meaningful for global-scoped inline assembly, remove it

error: the `label` operand cannot be used with `global_asm!`
--> $DIR/parse-error.rs:160:19
|
LL | global_asm!("{}", label {});
| ^^^^^^^^ expected operand, options, or additional template string
LL | global_asm!("{}", label(reg));
| ^^^^^ the `label` operand is not meaningful for global-scoped inline assembly, remove it

error[E0435]: attempt to use a non-constant value in a constant
--> $DIR/parse-error.rs:39:37
Expand Down Expand Up @@ -441,6 +471,6 @@ help: consider using `const` instead of `let`
LL | const bar: /* Type */ = 0;
| ~~~~~ ++++++++++++

error: aborting due to 67 previous errors
error: aborting due to 72 previous errors

For more information about this error, try `rustc --explain E0435`.
Loading