Rollup merge of #73227 - camelid:multiple-asm-options, r=Amanieu

Allow multiple `asm!` options groups and report an error on duplicate options

Fixes #73193

Cc @joshtriplett @Amanieu

- [x] Allow multiple options
- [x] Update existing test
- [x] Add new tests
- [x] Check for duplicate options
- [x] Add duplicate options tests
- [x] Finalize suggestion format for duplicate options error

7 files changed, 236 insertions, 67 deletions

diff --git a/src/librustc_builtin_macros/asm.rs b/src/librustc_builtin_macros/asm.rs
index 29885679604..52f86aa7e06 100644
--- a/src/librustc_builtin_macros/asm.rs
+++ b/src/librustc_builtin_macros/asm.rs
@@ -16,7 +16,7 @@ struct AsmArgs {
     named_args: FxHashMap<Symbol, usize>,
     reg_args: FxHashSet<usize>,
     options: ast::InlineAsmOptions,
-    options_span: Option<Span>,
+    options_spans: Vec<Span>,
 }
 
 fn parse_args<'a>(
@@ -59,7 +59,7 @@ fn parse_args<'a>(
         named_args: FxHashMap::default(),
         reg_args: FxHashSet::default(),
         options: ast::InlineAsmOptions::empty(),
-        options_span: None,
+        options_spans: vec![],
     };
 
     let mut allow_templates = true;
@@ -174,9 +174,9 @@ fn parse_args<'a>(
 
         // Validate the order of named, positional & explicit register operands and options. We do
         // this at the end once we have the full span of the argument available.
-        if let Some(options_span) = args.options_span {
+        if !args.options_spans.is_empty() {
             ecx.struct_span_err(span, "arguments are not allowed after options")
-                .span_label(options_span, "previous options")
+                .span_labels(args.options_spans.clone(), "previous options")
                 .span_label(span, "argument")
                 .emit();
         }
@@ -227,23 +227,23 @@ fn parse_args<'a>(
     if args.options.contains(ast::InlineAsmOptions::NOMEM)
         && args.options.contains(ast::InlineAsmOptions::READONLY)
     {
-        let span = args.options_span.unwrap();
-        ecx.struct_span_err(span, "the `nomem` and `readonly` options are mutually exclusive")
+        let spans = args.options_spans.clone();
+        ecx.struct_span_err(spans, "the `nomem` and `readonly` options are mutually exclusive")
             .emit();
     }
     if args.options.contains(ast::InlineAsmOptions::PURE)
         && args.options.contains(ast::InlineAsmOptions::NORETURN)
     {
-        let span = args.options_span.unwrap();
-        ecx.struct_span_err(span, "the `pure` and `noreturn` options are mutually exclusive")
+        let spans = args.options_spans.clone();
+        ecx.struct_span_err(spans, "the `pure` and `noreturn` options are mutually exclusive")
             .emit();
     }
     if args.options.contains(ast::InlineAsmOptions::PURE)
         && !args.options.intersects(ast::InlineAsmOptions::NOMEM | ast::InlineAsmOptions::READONLY)
     {
-        let span = args.options_span.unwrap();
+        let spans = args.options_spans.clone();
         ecx.struct_span_err(
-            span,
+            spans,
             "the `pure` option must be combined with either `nomem` or `readonly`",
         )
         .emit();
@@ -267,7 +267,7 @@ fn parse_args<'a>(
     }
     if args.options.contains(ast::InlineAsmOptions::PURE) && !have_real_output {
         ecx.struct_span_err(
-            args.options_span.unwrap(),
+            args.options_spans.clone(),
             "asm with `pure` option must have at least one output",
         )
         .emit();
@@ -283,6 +283,50 @@ fn parse_args<'a>(
     Ok(args)
 }
 
+/// Report a duplicate option error.
+///
+/// This function must be called immediately after the option token is parsed.
+/// Otherwise, the suggestion will be incorrect.
+fn err_duplicate_option<'a>(p: &mut Parser<'a>, symbol: Symbol, span: Span) {
+    let mut err = p
+        .sess
+        .span_diagnostic
+        .struct_span_err(span, &format!("the `{}` option was already provided", symbol));
+    err.span_label(span, "this option was already provided");
+
+    // Tool-only output
+    let mut full_span = span;
+    if p.token.kind == token::Comma {
+        full_span = full_span.to(p.token.span);
+    }
+    err.tool_only_span_suggestion(
+        full_span,
+        "remove this option",
+        String::new(),
+        Applicability::MachineApplicable,
+    );
+
+    err.emit();
+}
+
+/// Try to set the provided option in the provided `AsmArgs`.
+/// If it is already set, report a duplicate option error.
+///
+/// This function must be called immediately after the option token is parsed.
+/// Otherwise, the error will not point to the correct spot.
+fn try_set_option<'a>(
+    p: &mut Parser<'a>,
+    args: &mut AsmArgs,
+    symbol: Symbol,
+    option: ast::InlineAsmOptions,
+) {
+    if !args.options.contains(option) {
+        args.options |= option;
+    } else {
+        err_duplicate_option(p, symbol, p.prev_token.span);
+    }
+}
+
 fn parse_options<'a>(p: &mut Parser<'a>, args: &mut AsmArgs) -> Result<(), DiagnosticBuilder<'a>> {
     let span_start = p.prev_token.span;
 
@@ -290,20 +334,20 @@ fn parse_options<'a>(p: &mut Parser<'a>, args: &mut AsmArgs) -> Result<(), Diagn
 
     while !p.eat(&token::CloseDelim(token::DelimToken::Paren)) {
         if p.eat(&token::Ident(sym::pure, false)) {
-            args.options |= ast::InlineAsmOptions::PURE;
+            try_set_option(p, args, sym::pure, ast::InlineAsmOptions::PURE);
         } else if p.eat(&token::Ident(sym::nomem, false)) {
-            args.options |= ast::InlineAsmOptions::NOMEM;
+            try_set_option(p, args, sym::nomem, ast::InlineAsmOptions::NOMEM);
         } else if p.eat(&token::Ident(sym::readonly, false)) {
-            args.options |= ast::InlineAsmOptions::READONLY;
+            try_set_option(p, args, sym::readonly, ast::InlineAsmOptions::READONLY);
         } else if p.eat(&token::Ident(sym::preserves_flags, false)) {
-            args.options |= ast::InlineAsmOptions::PRESERVES_FLAGS;
+            try_set_option(p, args, sym::preserves_flags, ast::InlineAsmOptions::PRESERVES_FLAGS);
         } else if p.eat(&token::Ident(sym::noreturn, false)) {
-            args.options |= ast::InlineAsmOptions::NORETURN;
+            try_set_option(p, args, sym::noreturn, ast::InlineAsmOptions::NORETURN);
         } else if p.eat(&token::Ident(sym::nostack, false)) {
-            args.options |= ast::InlineAsmOptions::NOSTACK;
+            try_set_option(p, args, sym::nostack, ast::InlineAsmOptions::NOSTACK);
         } else {
             p.expect(&token::Ident(sym::att_syntax, false))?;
-            args.options |= ast::InlineAsmOptions::ATT_SYNTAX;
+            try_set_option(p, args, sym::att_syntax, ast::InlineAsmOptions::ATT_SYNTAX);
         }
 
         // Allow trailing commas
@@ -314,14 +358,7 @@ fn parse_options<'a>(p: &mut Parser<'a>, args: &mut AsmArgs) -> Result<(), Diagn
     }
 
     let new_span = span_start.to(p.prev_token.span);
-    if let Some(options_span) = args.options_span {
-        p.struct_span_err(new_span, "asm options cannot be specified multiple times")
-            .span_label(options_span, "previously here")
-            .span_label(new_span, "duplicate options")
-            .emit();
-    } else {
-        args.options_span = Some(new_span);
-    }
+    args.options_spans.push(new_span);
 
     Ok(())
 }
diff --git a/src/test/codegen/asm-multiple-options.rs b/src/test/codegen/asm-multiple-options.rs
new file mode 100644
index 00000000000..c702742bf1a
--- /dev/null
+++ b/src/test/codegen/asm-multiple-options.rs
@@ -0,0 +1,53 @@
+// compile-flags: -O
+// only-x86_64
+
+#![crate_type = "rlib"]
+#![feature(asm)]
+
+// CHECK-LABEL: @pure
+// CHECK-NOT: asm
+// CHECK: ret void
+#[no_mangle]
+pub unsafe fn pure(x: i32) {
+    let y: i32;
+    asm!("", out("ax") y, in("bx") x, options(pure), options(nomem));
+}
+
+pub static mut VAR: i32 = 0;
+pub static mut DUMMY_OUTPUT: i32 = 0;
+
+// CHECK-LABEL: @readonly
+// CHECK: call i32 asm
+// CHECK: ret i32 1
+#[no_mangle]
+pub unsafe fn readonly() -> i32 {
+    VAR = 1;
+    asm!("", out("ax") DUMMY_OUTPUT, options(pure), options(readonly));
+    VAR
+}
+
+// CHECK-LABEL: @nomem
+// CHECK-NOT: store
+// CHECK: call i32 asm
+// CHECK: store
+// CHECK: ret i32 2
+#[no_mangle]
+pub unsafe fn nomem() -> i32 {
+    VAR = 1;
+    asm!("", out("ax") DUMMY_OUTPUT, options(pure), options(nomem));
+    VAR = 2;
+    VAR
+}
+
+// CHECK-LABEL: @not_nomem
+// CHECK: store
+// CHECK: call i32 asm
+// CHECK: store
+// CHECK: ret i32 2
+#[no_mangle]
+pub unsafe fn not_nomem() -> i32 {
+    VAR = 1;
+    asm!("", out("ax") DUMMY_OUTPUT, options(pure), options(readonly));
+    VAR = 2;
+    VAR
+}
diff --git a/src/test/ui/asm/duplicate-options.fixed b/src/test/ui/asm/duplicate-options.fixed
new file mode 100644
index 00000000000..f4672a50fd0
--- /dev/null
+++ b/src/test/ui/asm/duplicate-options.fixed
@@ -0,0 +1,26 @@
+// only-x86_64
+// run-rustfix
+
+#![feature(asm)]
+
+fn main() {
+    unsafe {
+        asm!("", options(nomem, ));
+        //~^ ERROR the `nomem` option was already provided
+        asm!("", options(att_syntax, ));
+        //~^ ERROR the `att_syntax` option was already provided
+        asm!("", options(nostack, att_syntax), options());
+        //~^ ERROR the `nostack` option was already provided
+        asm!("", options(nostack, ), options(), options());
+        //~^ ERROR the `nostack` option was already provided
+        //~| ERROR the `nostack` option was already provided
+        //~| ERROR the `nostack` option was already provided
+        asm!(
+            "",
+            options(nomem, noreturn),
+            options(att_syntax, ), //~ ERROR the `noreturn` option was already provided
+            options( nostack), //~ ERROR the `nomem` option was already provided
+            options(), //~ ERROR the `noreturn` option was already provided
+        );
+    }
+}
diff --git a/src/test/ui/asm/duplicate-options.rs b/src/test/ui/asm/duplicate-options.rs
new file mode 100644
index 00000000000..80292d7521a
--- /dev/null
+++ b/src/test/ui/asm/duplicate-options.rs
@@ -0,0 +1,26 @@
+// only-x86_64
+// run-rustfix
+
+#![feature(asm)]
+
+fn main() {
+    unsafe {
+        asm!("", options(nomem, nomem));
+        //~^ ERROR the `nomem` option was already provided
+        asm!("", options(att_syntax, att_syntax));
+        //~^ ERROR the `att_syntax` option was already provided
+        asm!("", options(nostack, att_syntax), options(nostack));
+        //~^ ERROR the `nostack` option was already provided
+        asm!("", options(nostack, nostack), options(nostack), options(nostack));
+        //~^ ERROR the `nostack` option was already provided
+        //~| ERROR the `nostack` option was already provided
+        //~| ERROR the `nostack` option was already provided
+        asm!(
+            "",
+            options(nomem, noreturn),
+            options(att_syntax, noreturn), //~ ERROR the `noreturn` option was already provided
+            options(nomem, nostack), //~ ERROR the `nomem` option was already provided
+            options(noreturn), //~ ERROR the `noreturn` option was already provided
+        );
+    }
+}
diff --git a/src/test/ui/asm/duplicate-options.stderr b/src/test/ui/asm/duplicate-options.stderr
new file mode 100644
index 00000000000..cd8d743e031
--- /dev/null
+++ b/src/test/ui/asm/duplicate-options.stderr
@@ -0,0 +1,56 @@
+error: the `nomem` option was already provided
+  --> $DIR/duplicate-options.rs:8:33
+   |
+LL |         asm!("", options(nomem, nomem));
+   |                                 ^^^^^ this option was already provided
+
+error: the `att_syntax` option was already provided
+  --> $DIR/duplicate-options.rs:10:38
+   |
+LL |         asm!("", options(att_syntax, att_syntax));
+   |                                      ^^^^^^^^^^ this option was already provided
+
+error: the `nostack` option was already provided
+  --> $DIR/duplicate-options.rs:12:56
+   |
+LL |         asm!("", options(nostack, att_syntax), options(nostack));
+   |                                                        ^^^^^^^ this option was already provided
+
+error: the `nostack` option was already provided
+  --> $DIR/duplicate-options.rs:14:35
+   |
+LL |         asm!("", options(nostack, nostack), options(nostack), options(nostack));
+   |                                   ^^^^^^^ this option was already provided
+
+error: the `nostack` option was already provided
+  --> $DIR/duplicate-options.rs:14:53
+   |
+LL |         asm!("", options(nostack, nostack), options(nostack), options(nostack));
+   |                                                     ^^^^^^^ this option was already provided
+
+error: the `nostack` option was already provided
+  --> $DIR/duplicate-options.rs:14:71
+   |
+LL |         asm!("", options(nostack, nostack), options(nostack), options(nostack));
+   |                                                                       ^^^^^^^ this option was already provided
+
+error: the `noreturn` option was already provided
+  --> $DIR/duplicate-options.rs:21:33
+   |
+LL |             options(att_syntax, noreturn),
+   |                                 ^^^^^^^^ this option was already provided
+
+error: the `nomem` option was already provided
+  --> $DIR/duplicate-options.rs:22:21
+   |
+LL |             options(nomem, nostack),
+   |                     ^^^^^ this option was already provided
+
+error: the `noreturn` option was already provided
+  --> $DIR/duplicate-options.rs:23:21
+   |
+LL |             options(noreturn),
+   |                     ^^^^^^^^ this option was already provided
+
+error: aborting due to 9 previous errors
+
diff --git a/src/test/ui/asm/parse-error.rs b/src/test/ui/asm/parse-error.rs
index fbf399d8b07..538a3fde8fd 100644
--- a/src/test/ui/asm/parse-error.rs
+++ b/src/test/ui/asm/parse-error.rs
@@ -34,11 +34,6 @@ fn main() {
         //~^ ERROR expected one of
         asm!("", options(nomem, foo));
         //~^ ERROR expected one of
-        asm!("", options(), options());
-        //~^ ERROR asm options cannot be specified multiple times
-        asm!("", options(), options(), options());
-        //~^ ERROR asm options cannot be specified multiple times
-        //~^^ ERROR asm options cannot be specified multiple times
         asm!("{}", options(), const foo);
         //~^ ERROR arguments are not allowed after options
         asm!("{a}", a = const foo, a = const bar);
diff --git a/src/test/ui/asm/parse-error.stderr b/src/test/ui/asm/parse-error.stderr
index ba7e8f7a03c..dfbfc0abe34 100644
--- a/src/test/ui/asm/parse-error.stderr
+++ b/src/test/ui/asm/parse-error.stderr
@@ -82,32 +82,8 @@ error: expected one of `)`, `att_syntax`, `nomem`, `noreturn`, `nostack`, `prese
 LL |         asm!("", options(nomem, foo));
    |                                 ^^^ expected one of 8 possible tokens
 
-error: asm options cannot be specified multiple times
-  --> $DIR/parse-error.rs:37:29
-   |
-LL |         asm!("", options(), options());
-   |                  ---------  ^^^^^^^^^ duplicate options
-   |                  |
-   |                  previously here
-
-error: asm options cannot be specified multiple times
-  --> $DIR/parse-error.rs:39:29
-   |
-LL |         asm!("", options(), options(), options());
-   |                  ---------  ^^^^^^^^^ duplicate options
-   |                  |
-   |                  previously here
-
-error: asm options cannot be specified multiple times
-  --> $DIR/parse-error.rs:39:40
-   |
-LL |         asm!("", options(), options(), options());
-   |                  ---------             ^^^^^^^^^ duplicate options
-   |                  |
-   |                  previously here
-
 error: arguments are not allowed after options
-  --> $DIR/parse-error.rs:42:31
+  --> $DIR/parse-error.rs:37:31
    |
 LL |         asm!("{}", options(), const foo);
    |                    ---------  ^^^^^^^^^ argument
@@ -115,7 +91,7 @@ LL |         asm!("{}", options(), const foo);
    |                    previous options
 
 error: duplicate argument named `a`
-  --> $DIR/parse-error.rs:44:36
+  --> $DIR/parse-error.rs:39:36
    |
 LL |         asm!("{a}", a = const foo, a = const bar);
    |                     -------------  ^^^^^^^^^^^^^ duplicate argument
@@ -123,7 +99,7 @@ LL |         asm!("{a}", a = const foo, a = const bar);
    |                     previously here
 
 error: argument never used
-  --> $DIR/parse-error.rs:44:36
+  --> $DIR/parse-error.rs:39:36
    |
 LL |         asm!("{a}", a = const foo, a = const bar);
    |                                    ^^^^^^^^^^^^^ argument never used
@@ -131,13 +107,13 @@ LL |         asm!("{a}", a = const foo, a = const bar);
    = help: if this argument is intentionally unused, consider using it in an asm comment: `"/* {1} */"`
 
 error: explicit register arguments cannot have names
-  --> $DIR/parse-error.rs:47:18
+  --> $DIR/parse-error.rs:42:18
    |
 LL |         asm!("", a = in("eax") foo);
    |                  ^^^^^^^^^^^^^^^^^
 
 error: named arguments cannot follow explicit register arguments
-  --> $DIR/parse-error.rs:49:36
+  --> $DIR/parse-error.rs:44:36
    |
 LL |         asm!("{a}", in("eax") foo, a = const bar);
    |                     -------------  ^^^^^^^^^^^^^ named argument
@@ -145,7 +121,7 @@ LL |         asm!("{a}", in("eax") foo, a = const bar);
    |                     explicit register argument
 
 error: named arguments cannot follow explicit register arguments
-  --> $DIR/parse-error.rs:51:36
+  --> $DIR/parse-error.rs:46:36
    |
 LL |         asm!("{a}", in("eax") foo, a = const bar);
    |                     -------------  ^^^^^^^^^^^^^ named argument
@@ -153,7 +129,7 @@ LL |         asm!("{a}", in("eax") foo, a = const bar);
    |                     explicit register argument
 
 error: positional arguments cannot follow named arguments or explicit register arguments
-  --> $DIR/parse-error.rs:53:36
+  --> $DIR/parse-error.rs:48:36
    |
 LL |         asm!("{1}", in("eax") foo, const bar);
    |                     -------------  ^^^^^^^^^ positional argument
@@ -161,19 +137,19 @@ LL |         asm!("{1}", in("eax") foo, const bar);
    |                     explicit register argument
 
 error: expected one of `const`, `in`, `inlateout`, `inout`, `lateout`, `options`, `out`, or `sym`, found `""`
-  --> $DIR/parse-error.rs:55:29
+  --> $DIR/parse-error.rs:50:29
    |
 LL |         asm!("", options(), "");
    |                             ^^ expected one of 8 possible tokens
 
 error: expected one of `const`, `in`, `inlateout`, `inout`, `lateout`, `options`, `out`, or `sym`, found `"{}"`
-  --> $DIR/parse-error.rs:57:33
+  --> $DIR/parse-error.rs:52:33
    |
 LL |         asm!("{}", in(reg) foo, "{}", out(reg) foo);
    |                                 ^^^^ expected one of 8 possible tokens
 
 error: asm template must be a string literal
-  --> $DIR/parse-error.rs:59:14
+  --> $DIR/parse-error.rs:54:14
    |
 LL |         asm!(format!("{{{}}}", 0), in(reg) foo);
    |              ^^^^^^^^^^^^^^^^^^^^
@@ -181,12 +157,12 @@ LL |         asm!(format!("{{{}}}", 0), in(reg) foo);
    = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error: asm template must be a string literal
-  --> $DIR/parse-error.rs:61:21
+  --> $DIR/parse-error.rs:56:21
    |
 LL |         asm!("{1}", format!("{{{}}}", 0), in(reg) foo, out(reg) bar);
    |                     ^^^^^^^^^^^^^^^^^^^^
    |
    = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
 
-error: aborting due to 28 previous errors
+error: aborting due to 25 previous errors