From 5bf2f50e7b6751e6c3901b155aa25e95b8d53cd8 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Sat, 6 Jun 2020 11:37:08 +0100 Subject: [PATCH 1/4] Allow unused arguments in asm! --- .../unstable-book/src/library-features/asm.md | 6 ++- src/librustc_builtin_macros/asm.rs | 34 +++----------- src/librustc_session/lint/builtin.rs | 6 +++ src/test/ui/asm/bad-template.rs | 8 ++-- src/test/ui/asm/bad-template.stderr | 46 +++++++++++-------- src/test/ui/asm/parse-error.rs | 2 +- src/test/ui/asm/parse-error.stderr | 16 ++++--- 7 files changed, 58 insertions(+), 60 deletions(-) diff --git a/src/doc/unstable-book/src/library-features/asm.md b/src/doc/unstable-book/src/library-features/asm.md index ea560a6d70915..fbb40f1d2f3d4 100644 --- a/src/doc/unstable-book/src/library-features/asm.md +++ b/src/doc/unstable-book/src/library-features/asm.md @@ -201,7 +201,7 @@ fn mul(a: u64, b: u64) -> u128 { ); } - (hi as u128) << 64 + lo as u128 + ((hi as u128) << 64) + lo as u128 } ``` @@ -382,7 +382,9 @@ The macro will initially be supported only on ARM, AArch64, x86, x86-64 and RISC The assembler template uses the same syntax as [format strings][format-syntax] (i.e. placeholders are specified by curly braces). The corresponding arguments are accessed in order, by index, or by name. However, implicit named arguments (introduced by [RFC #2795][rfc-2795]) are not supported. -As with format strings, named arguments must appear after positional arguments. Explicit register operands must appear at the end of the operand list, after any named arguments if any. Explicit register operands cannot be used by placeholders in the template string. All other operands must appear at least once in the template string, otherwise a compiler error is generated. +As with format strings, named arguments must appear after positional arguments. Explicit register operands must appear at the end of the operand list, after named arguments if any. + +Explicit register operands cannot be used by placeholders in the template string. All other named and positional operands must appear at least once in the template string, otherwise a compiler error is generated. The exact assembly code syntax is target-specific and opaque to the compiler except for the way operands are substituted into the template string to form the code passed to the assembler. diff --git a/src/librustc_builtin_macros/asm.rs b/src/librustc_builtin_macros/asm.rs index aabd5b5b5c31b..c1c4f846dc778 100644 --- a/src/librustc_builtin_macros/asm.rs +++ b/src/librustc_builtin_macros/asm.rs @@ -3,10 +3,11 @@ use rustc_ast::ptr::P; use rustc_ast::token; use rustc_ast::tokenstream::TokenStream; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; -use rustc_errors::{Applicability, DiagnosticBuilder}; +use rustc_errors::{pluralize, Applicability, DiagnosticBuilder}; use rustc_expand::base::{self, *}; use rustc_parse::parser::Parser; use rustc_parse_format as parse; +use rustc_session::lint::builtin::UNUSED_ASM_ARGUMENTS; use rustc_span::symbol::{kw, sym, Symbol}; use rustc_span::{InnerSpan, Span}; @@ -485,34 +486,11 @@ fn expand_preparsed_asm(ecx: &mut ExtCtxt<'_>, sp: Span, args: AsmArgs) -> P {} - 1 => { - let (sp, msg) = unused_operands.into_iter().next().unwrap(); - let mut err = ecx.struct_span_err(sp, msg); - err.span_label(sp, msg); - err.emit(); - } - _ => { - let mut err = ecx.struct_span_err( - unused_operands.iter().map(|&(sp, _)| sp).collect::>(), - "multiple unused asm arguments", - ); - for (sp, msg) in unused_operands { - err.span_label(sp, msg); - } - err.emit(); - } + if !unused_operands.is_empty() { + let msg = format!("asm argument{} not used in template", pluralize!(unused_operands.len())); + ecx.parse_sess.buffer_lint(UNUSED_ASM_ARGUMENTS, unused_operands, ast::CRATE_NODE_ID, &msg); } let line_spans = if parser.line_spans.is_empty() { diff --git a/src/librustc_session/lint/builtin.rs b/src/librustc_session/lint/builtin.rs index 7112ac35b082b..cade74da0034c 100644 --- a/src/librustc_session/lint/builtin.rs +++ b/src/librustc_session/lint/builtin.rs @@ -532,6 +532,12 @@ declare_lint! { "unsafe operations in unsafe functions without an explicit unsafe block are deprecated", } +declare_lint! { + pub UNUSED_ASM_ARGUMENTS, + Warn, + "inline asm arguments not used in the template string", +} + declare_lint_pass! { /// Does nothing as a lint pass, but registers some `Lint`s /// that are used by other parts of the compiler. diff --git a/src/test/ui/asm/bad-template.rs b/src/test/ui/asm/bad-template.rs index 0b333eca1ab91..618aa69ee96c4 100644 --- a/src/test/ui/asm/bad-template.rs +++ b/src/test/ui/asm/bad-template.rs @@ -9,18 +9,20 @@ fn main() { //~^ ERROR invalid reference to argument at index 0 asm!("{1}", in(reg) foo); //~^ ERROR invalid reference to argument at index 1 - //~^^ ERROR argument never used + //~^^ WARN asm argument not used in template asm!("{a}"); //~^ ERROR there is no argument named `a` asm!("{}", a = in(reg) foo); //~^ ERROR invalid reference to argument at index 0 - //~^^ ERROR argument never used + //~^^ WARN asm argument not used in template asm!("{1}", a = in(reg) foo); //~^ ERROR invalid reference to argument at index 1 - //~^^ ERROR named argument never used + //~^^ WARN asm argument not used in template asm!("{}", in("eax") foo); //~^ ERROR invalid reference to argument at index 0 asm!("{:foo}", in(reg) foo); //~^ ERROR asm template modifier must be a single character + asm!("", in(reg) 0, in(reg) 1); + //~^ WARN asm arguments not used in template } } diff --git a/src/test/ui/asm/bad-template.stderr b/src/test/ui/asm/bad-template.stderr index 2de76ef824192..5e2b717839abc 100644 --- a/src/test/ui/asm/bad-template.stderr +++ b/src/test/ui/asm/bad-template.stderr @@ -14,12 +14,6 @@ LL | asm!("{1}", in(reg) foo); | = note: there is 1 argument -error: argument never used - --> $DIR/bad-template.rs:10:21 - | -LL | asm!("{1}", in(reg) foo); - | ^^^^^^^^^^^ argument never used - error: there is no argument named `a` --> $DIR/bad-template.rs:13:15 | @@ -41,12 +35,6 @@ note: named arguments cannot be referenced by position LL | asm!("{}", a = in(reg) foo); | ^^^^^^^^^^^^^^^ -error: named argument never used - --> $DIR/bad-template.rs:15:20 - | -LL | asm!("{}", a = in(reg) foo); - | ^^^^^^^^^^^^^^^ named argument never used - error: invalid reference to argument at index 1 --> $DIR/bad-template.rs:18:15 | @@ -55,12 +43,6 @@ LL | asm!("{1}", a = in(reg) foo); | = note: no positional arguments were given -error: named argument never used - --> $DIR/bad-template.rs:18:21 - | -LL | asm!("{1}", a = in(reg) foo); - | ^^^^^^^^^^^^^^^ named argument never used - error: invalid reference to argument at index 0 --> $DIR/bad-template.rs:21:15 | @@ -82,5 +64,31 @@ error: asm template modifier must be a single character LL | asm!("{:foo}", in(reg) foo); | ^^^ -error: aborting due to 10 previous errors +warning: asm argument not used in template + --> $DIR/bad-template.rs:10:21 + | +LL | asm!("{1}", in(reg) foo); + | ^^^^^^^^^^^ + | + = note: `#[warn(unused_asm_arguments)]` on by default + +warning: asm argument not used in template + --> $DIR/bad-template.rs:15:20 + | +LL | asm!("{}", a = in(reg) foo); + | ^^^^^^^^^^^^^^^ + +warning: asm argument not used in template + --> $DIR/bad-template.rs:18:21 + | +LL | asm!("{1}", a = in(reg) foo); + | ^^^^^^^^^^^^^^^ + +warning: asm arguments not used in template + --> $DIR/bad-template.rs:25:18 + | +LL | asm!("", in(reg) 0, in(reg) 1); + | ^^^^^^^^^ ^^^^^^^^^ + +error: aborting due to 7 previous errors; 4 warnings emitted diff --git a/src/test/ui/asm/parse-error.rs b/src/test/ui/asm/parse-error.rs index 2b1f018f3642e..66e788501570b 100644 --- a/src/test/ui/asm/parse-error.rs +++ b/src/test/ui/asm/parse-error.rs @@ -43,7 +43,7 @@ fn main() { //~^ ERROR arguments are not allowed after options asm!("{a}", a = const foo, a = const bar); //~^ ERROR duplicate argument named `a` - //~^^ ERROR argument never used + //~^^ WARN asm argument not used in template asm!("", a = in("eax") foo); //~^ ERROR explicit register arguments cannot have names asm!("{a}", in("eax") foo, a = const bar); diff --git a/src/test/ui/asm/parse-error.stderr b/src/test/ui/asm/parse-error.stderr index fa422f56bece5..433c89b42e699 100644 --- a/src/test/ui/asm/parse-error.stderr +++ b/src/test/ui/asm/parse-error.stderr @@ -122,12 +122,6 @@ LL | asm!("{a}", a = const foo, a = const bar); | | | previously here -error: argument never used - --> $DIR/parse-error.rs:44:36 - | -LL | asm!("{a}", a = const foo, a = const bar); - | ^^^^^^^^^^^^^ argument never used - error: explicit register arguments cannot have names --> $DIR/parse-error.rs:47:18 | @@ -158,5 +152,13 @@ LL | asm!("{1}", in("eax") foo, const bar); | | | explicit register argument -error: aborting due to 24 previous errors +warning: asm argument not used in template + --> $DIR/parse-error.rs:44:36 + | +LL | asm!("{a}", a = const foo, a = const bar); + | ^^^^^^^^^^^^^ + | + = note: `#[warn(unused_asm_arguments)]` on by default + +error: aborting due to 23 previous errors; 1 warning emitted From 01ef7f39c531ff5b508126cbd403981d518c3734 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Wed, 10 Jun 2020 07:28:51 +0100 Subject: [PATCH 2/4] Update docs --- src/doc/unstable-book/src/library-features/asm.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/doc/unstable-book/src/library-features/asm.md b/src/doc/unstable-book/src/library-features/asm.md index fbb40f1d2f3d4..ed5ca27f0d283 100644 --- a/src/doc/unstable-book/src/library-features/asm.md +++ b/src/doc/unstable-book/src/library-features/asm.md @@ -384,7 +384,7 @@ The assembler template uses the same syntax as [format strings][format-syntax] ( As with format strings, named arguments must appear after positional arguments. Explicit register operands must appear at the end of the operand list, after named arguments if any. -Explicit register operands cannot be used by placeholders in the template string. All other named and positional operands must appear at least once in the template string, otherwise a compiler error is generated. +Explicit register operands cannot be used by placeholders in the template string. All other named and positional operands must appear at least once in the template string, otherwise the compiler will emit a warning. The exact assembly code syntax is target-specific and opaque to the compiler except for the way operands are substituted into the template string to form the code passed to the assembler. From 60194269c5b98ba480cf5c3aed6ad1fd8914880b Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Wed, 10 Jun 2020 07:48:54 +0100 Subject: [PATCH 3/4] Add to hardwired lints --- src/librustc_session/lint/builtin.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/librustc_session/lint/builtin.rs b/src/librustc_session/lint/builtin.rs index cade74da0034c..9e5505695d1bc 100644 --- a/src/librustc_session/lint/builtin.rs +++ b/src/librustc_session/lint/builtin.rs @@ -610,6 +610,7 @@ declare_lint_pass! { INLINE_NO_SANITIZE, ASM_SUB_REGISTER, UNSAFE_OP_IN_UNSAFE_FN, + UNUSED_ASM_ARGUMENTS, ] } From 7086653ead4ee64eab182d83ad847f8fe97c05af Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Wed, 10 Jun 2020 07:49:48 +0100 Subject: [PATCH 4/4] WIP: Move to AST lowering --- src/librustc_ast_lowering/expr.rs | 35 ++++++++++++++++++++++++++--- src/librustc_builtin_macros/asm.rs | 26 +++------------------ src/test/ui/asm/bad-template.rs | 2 ++ src/test/ui/asm/bad-template.stderr | 8 ++++++- 4 files changed, 44 insertions(+), 27 deletions(-) diff --git a/src/librustc_ast_lowering/expr.rs b/src/librustc_ast_lowering/expr.rs index c9037da377ebb..ae6ccf2beeaac 100644 --- a/src/librustc_ast_lowering/expr.rs +++ b/src/librustc_ast_lowering/expr.rs @@ -6,9 +6,10 @@ use rustc_ast::ptr::P as AstP; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_data_structures::thin_vec::ThinVec; -use rustc_errors::struct_span_err; +use rustc_errors::{pluralize, struct_span_err}; use rustc_hir as hir; use rustc_hir::def::Res; +use rustc_session::lint::builtin::UNUSED_ASM_ARGUMENTS; use rustc_span::source_map::{respan, DesugaringKind, Span, Spanned}; use rustc_span::symbol::{sym, Ident, Symbol}; use rustc_target::asm; @@ -179,7 +180,7 @@ impl<'hir> LoweringContext<'_, 'hir> { let e = e.as_ref().map(|x| self.lower_expr(x)); hir::ExprKind::Ret(e) } - ExprKind::InlineAsm(ref asm) => self.lower_expr_asm(e.span, asm), + ExprKind::InlineAsm(ref asm) => self.lower_expr_asm(e.span, e.id, asm), ExprKind::LlvmInlineAsm(ref asm) => self.lower_expr_llvm_asm(asm), ExprKind::Struct(ref path, ref fields, ref maybe_expr) => { let maybe_expr = maybe_expr.as_ref().map(|x| self.lower_expr(x)); @@ -973,7 +974,7 @@ impl<'hir> LoweringContext<'_, 'hir> { result } - fn lower_expr_asm(&mut self, sp: Span, asm: &InlineAsm) -> hir::ExprKind<'hir> { + fn lower_expr_asm(&mut self, sp: Span, id: NodeId, asm: &InlineAsm) -> hir::ExprKind<'hir> { if self.sess.asm_arch.is_none() { struct_span_err!(self.sess, sp, E0472, "asm! is unsupported on this target").emit(); } @@ -1265,6 +1266,34 @@ impl<'hir> LoweringContext<'_, 'hir> { } } + // Warn about operands that are not referenced in the template string. + let mut used = vec![false; operands.len()]; + for p in &asm.template { + if let InlineAsmTemplatePiece::Placeholder { operand_idx, modifier: _, span: _ } = *p { + used[operand_idx] = true; + } + } + let unused_operands: Vec<_> = used + .into_iter() + .enumerate() + .filter(|&(idx, used)| { + // Register operands are implicitly used since they are not allowed to be + // referenced in the template string. + !used && !matches!(operands[idx].reg(), Some(asm::InlineAsmRegOrRegClass::Reg(_))) + }) + .map(|(idx, _)| asm.operands[idx].1) + .collect(); + if !unused_operands.is_empty() { + let msg = + format!("asm argument{} not used in template", pluralize!(unused_operands.len())); + self.resolver.lint_buffer().buffer_lint( + UNUSED_ASM_ARGUMENTS, + id, + unused_operands, + &msg, + ); + } + let operands = self.arena.alloc_from_iter(operands); let template = self.arena.alloc_from_iter(asm.template.iter().cloned()); let line_spans = self.arena.alloc_slice(&asm.line_spans[..]); diff --git a/src/librustc_builtin_macros/asm.rs b/src/librustc_builtin_macros/asm.rs index c1c4f846dc778..297a187a0274c 100644 --- a/src/librustc_builtin_macros/asm.rs +++ b/src/librustc_builtin_macros/asm.rs @@ -3,11 +3,10 @@ use rustc_ast::ptr::P; use rustc_ast::token; use rustc_ast::tokenstream::TokenStream; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; -use rustc_errors::{pluralize, Applicability, DiagnosticBuilder}; +use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_expand::base::{self, *}; use rustc_parse::parser::Parser; use rustc_parse_format as parse; -use rustc_session::lint::builtin::UNUSED_ASM_ARGUMENTS; use rustc_span::symbol::{kw, sym, Symbol}; use rustc_span::{InnerSpan, Span}; @@ -385,13 +384,6 @@ fn expand_preparsed_asm(ecx: &mut ExtCtxt<'_>, sp: Span, args: AsmArgs) -> P = args.named_args.values().cloned().collect(); let mut arg_spans = parser.arg_places.iter().map(|span| template_span.from_inner(*span)); let mut template = vec![]; @@ -470,7 +462,6 @@ fn expand_preparsed_asm(ecx: &mut ExtCtxt<'_>, sp: Span, args: AsmArgs) -> P, sp: Span, args: AsmArgs) -> P = used - .into_iter() - .enumerate() - .filter(|&(_, used)| !used) - .map(|(idx, _)| operands[idx].1) - .collect(); - if !unused_operands.is_empty() { - let msg = format!("asm argument{} not used in template", pluralize!(unused_operands.len())); - ecx.parse_sess.buffer_lint(UNUSED_ASM_ARGUMENTS, unused_operands, ast::CRATE_NODE_ID, &msg); - } - let line_spans = if parser.line_spans.is_empty() { vec![template_sp] } else { parser.line_spans.iter().map(|span| template_span.from_inner(*span)).collect() }; - let inline_asm = ast::InlineAsm { template, operands, options: args.options, line_spans }; + let inline_asm = + ast::InlineAsm { template, operands: args.operands, options: args.options, line_spans }; P(ast::Expr { id: ast::DUMMY_NODE_ID, kind: ast::ExprKind::InlineAsm(P(inline_asm)), diff --git a/src/test/ui/asm/bad-template.rs b/src/test/ui/asm/bad-template.rs index 618aa69ee96c4..2c9ef02c776ad 100644 --- a/src/test/ui/asm/bad-template.rs +++ b/src/test/ui/asm/bad-template.rs @@ -24,5 +24,7 @@ fn main() { //~^ ERROR asm template modifier must be a single character asm!("", in(reg) 0, in(reg) 1); //~^ WARN asm arguments not used in template + #[allow(unused_asm_arguments)] + asm!("", in(reg) 0, in(reg) 1); } } diff --git a/src/test/ui/asm/bad-template.stderr b/src/test/ui/asm/bad-template.stderr index 5e2b717839abc..0d8d7782bd360 100644 --- a/src/test/ui/asm/bad-template.stderr +++ b/src/test/ui/asm/bad-template.stderr @@ -90,5 +90,11 @@ warning: asm arguments not used in template LL | asm!("", in(reg) 0, in(reg) 1); | ^^^^^^^^^ ^^^^^^^^^ -error: aborting due to 7 previous errors; 4 warnings emitted +warning: asm arguments not used in template + --> $DIR/bad-template.rs:28:18 + | +LL | asm!("", in(reg) 0, in(reg) 1); + | ^^^^^^^^^ ^^^^^^^^^ + +error: aborting due to 7 previous errors; 5 warnings emitted