From b94b7e7f1bf4e05d28ec7fca305c1d49f242c3dd Mon Sep 17 00:00:00 2001 From: Camelid Date: Sun, 14 Jun 2020 13:38:09 -0700 Subject: [PATCH] Make warning an error; use help instead of suggestion; clean up code For some reason, the help message is now in a separate message, which adds a lot of noise. I would like to try to get it back to one message. --- src/librustc_builtin_macros/asm.rs | 67 +++++++-------------- src/test/ui/asm/duplicate-options.rs | 19 +++--- src/test/ui/asm/duplicate-options.stderr | 74 ++++++++++++++++++------ 3 files changed, 88 insertions(+), 72 deletions(-) diff --git a/src/librustc_builtin_macros/asm.rs b/src/librustc_builtin_macros/asm.rs index b5641fdf2ab..922a06bae93 100644 --- a/src/librustc_builtin_macros/asm.rs +++ b/src/librustc_builtin_macros/asm.rs @@ -289,21 +289,24 @@ fn parse_args<'a>( Ok(args) } -fn warn_duplicate_option<'a>(p: &mut Parser<'a>, span: Span) { - let mut warn = if let Ok(snippet) = p.sess.source_map().span_to_snippet(span) { +fn err_duplicate_option<'a>(p: &mut Parser<'a>, span: Span) { + let mut err = if let Ok(snippet) = p.sess.source_map().span_to_snippet(span) { p.sess .span_diagnostic - .struct_span_warn(span, &format!("the `{}` option was already provided", snippet)) + .struct_span_err(span, &format!("the `{}` option was already provided", snippet)) } else { - p.sess.span_diagnostic.struct_span_warn(span, "this option was already provided") + p.sess.span_diagnostic.struct_span_err(span, "this option was already provided") }; - warn.span_suggestion( - span, - "remove this option", - String::new(), - Applicability::MachineApplicable, - ); - warn.emit(); + err.span_help(span, "remove this option"); + err.emit(); +} + +fn try_set_option<'a>(p: &mut Parser<'a>, args: &mut AsmArgs, option: ast::InlineAsmOptions) { + if !args.option_is_set(option) { + args.options |= option; + } else { + err_duplicate_option(p, p.prev_token.span); + } } fn parse_options<'a>(p: &mut Parser<'a>, args: &mut AsmArgs) -> Result<(), DiagnosticBuilder<'a>> { @@ -313,48 +316,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)) { - if !args.option_is_set(ast::InlineAsmOptions::PURE) { - args.options |= ast::InlineAsmOptions::PURE; - } else { - warn_duplicate_option(p, p.prev_token.span); - } + try_set_option(p, args, ast::InlineAsmOptions::PURE); } else if p.eat(&token::Ident(sym::nomem, false)) { - if !args.option_is_set(ast::InlineAsmOptions::NOMEM) { - args.options |= ast::InlineAsmOptions::NOMEM; - } else { - warn_duplicate_option(p, p.prev_token.span); - } + try_set_option(p, args, ast::InlineAsmOptions::NOMEM); } else if p.eat(&token::Ident(sym::readonly, false)) { - if !args.option_is_set(ast::InlineAsmOptions::READONLY) { - args.options |= ast::InlineAsmOptions::READONLY; - } else { - warn_duplicate_option(p, p.prev_token.span); - } + try_set_option(p, args, ast::InlineAsmOptions::READONLY); } else if p.eat(&token::Ident(sym::preserves_flags, false)) { - if !args.option_is_set(ast::InlineAsmOptions::PRESERVES_FLAGS) { - args.options |= ast::InlineAsmOptions::PRESERVES_FLAGS; - } else { - warn_duplicate_option(p, p.prev_token.span); - } + try_set_option(p, args, ast::InlineAsmOptions::PRESERVES_FLAGS); } else if p.eat(&token::Ident(sym::noreturn, false)) { - if !args.option_is_set(ast::InlineAsmOptions::NORETURN) { - args.options |= ast::InlineAsmOptions::NORETURN; - } else { - warn_duplicate_option(p, p.prev_token.span); - } + try_set_option(p, args, ast::InlineAsmOptions::NORETURN); } else if p.eat(&token::Ident(sym::nostack, false)) { - if !args.option_is_set(ast::InlineAsmOptions::NOSTACK) { - args.options |= ast::InlineAsmOptions::NOSTACK; - } else { - warn_duplicate_option(p, p.prev_token.span); - } + try_set_option(p, args, ast::InlineAsmOptions::NOSTACK); } else { p.expect(&token::Ident(sym::att_syntax, false))?; - if !args.option_is_set(ast::InlineAsmOptions::ATT_SYNTAX) { - args.options |= ast::InlineAsmOptions::ATT_SYNTAX; - } else { - warn_duplicate_option(p, p.prev_token.span); - } + try_set_option(p, args, ast::InlineAsmOptions::ATT_SYNTAX); } // Allow trailing commas diff --git a/src/test/ui/asm/duplicate-options.rs b/src/test/ui/asm/duplicate-options.rs index 9c447451511..e412932fa7e 100644 --- a/src/test/ui/asm/duplicate-options.rs +++ b/src/test/ui/asm/duplicate-options.rs @@ -1,19 +1,24 @@ // only-x86_64 -// build-pass #![feature(asm)] fn main() { unsafe { asm!("", options(nomem, nomem)); - //~^ WARNING the `nomem` option was already provided + //~^ ERROR the `nomem` option was already provided + //~| HELP remove this option asm!("", options(att_syntax, att_syntax)); - //~^ WARNING the `att_syntax` option was already provided + //~^ ERROR the `att_syntax` option was already provided + //~| HELP remove this option asm!("", options(nostack, att_syntax), options(nostack)); - //~^ WARNING the `nostack` option was already provided + //~^ ERROR the `nostack` option was already provided + //~| HELP remove this option asm!("", options(nostack, nostack), options(nostack), options(nostack)); - //~^ WARNING the `nostack` option was already provided - //~| WARNING the `nostack` option was already provided - //~| WARNING the `nostack` option was already provided + //~^ ERROR the `nostack` option was already provided + //~| HELP remove this option + //~| ERROR the `nostack` option was already provided + //~| HELP remove this option + //~| ERROR the `nostack` option was already provided + //~| HELP remove this option } } diff --git a/src/test/ui/asm/duplicate-options.stderr b/src/test/ui/asm/duplicate-options.stderr index 113aca8da90..accfdef474b 100644 --- a/src/test/ui/asm/duplicate-options.stderr +++ b/src/test/ui/asm/duplicate-options.stderr @@ -1,38 +1,74 @@ -warning: the `nomem` option was already provided - --> $DIR/duplicate-options.rs:8:33 +error: the `nomem` option was already provided + --> $DIR/duplicate-options.rs:7:33 | LL | asm!("", options(nomem, nomem)); - | ^^^^^ help: remove this option + | ^^^^^ + | +help: remove this option + --> $DIR/duplicate-options.rs:7:33 + | +LL | asm!("", options(nomem, nomem)); + | ^^^^^ -warning: the `att_syntax` 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)); - | ^^^^^^^^^^ help: remove this option + | ^^^^^^^^^^ + | +help: remove this option + --> $DIR/duplicate-options.rs:10:38 + | +LL | asm!("", options(att_syntax, att_syntax)); + | ^^^^^^^^^^ -warning: the `nostack` option was already provided - --> $DIR/duplicate-options.rs:12:56 +error: the `nostack` option was already provided + --> $DIR/duplicate-options.rs:13:56 | LL | asm!("", options(nostack, att_syntax), options(nostack)); - | ^^^^^^^ help: remove this option + | ^^^^^^^ + | +help: remove this option + --> $DIR/duplicate-options.rs:13:56 + | +LL | asm!("", options(nostack, att_syntax), options(nostack)); + | ^^^^^^^ -warning: the `nostack` option was already provided - --> $DIR/duplicate-options.rs:14:35 +error: the `nostack` option was already provided + --> $DIR/duplicate-options.rs:16:35 | LL | asm!("", options(nostack, nostack), options(nostack), options(nostack)); - | ^^^^^^^ help: remove this option - -warning: the `nostack` option was already provided - --> $DIR/duplicate-options.rs:14:53 + | ^^^^^^^ + | +help: remove this option + --> $DIR/duplicate-options.rs:16:35 | LL | asm!("", options(nostack, nostack), options(nostack), options(nostack)); - | ^^^^^^^ help: remove this option + | ^^^^^^^ -warning: the `nostack` option was already provided - --> $DIR/duplicate-options.rs:14:71 +error: the `nostack` option was already provided + --> $DIR/duplicate-options.rs:16:53 | LL | asm!("", options(nostack, nostack), options(nostack), options(nostack)); - | ^^^^^^^ help: remove this option + | ^^^^^^^ + | +help: remove this option + --> $DIR/duplicate-options.rs:16:53 + | +LL | asm!("", options(nostack, nostack), options(nostack), options(nostack)); + | ^^^^^^^ -warning: 6 warnings emitted +error: the `nostack` option was already provided + --> $DIR/duplicate-options.rs:16:71 + | +LL | asm!("", options(nostack, nostack), options(nostack), options(nostack)); + | ^^^^^^^ + | +help: remove this option + --> $DIR/duplicate-options.rs:16:71 + | +LL | asm!("", options(nostack, nostack), options(nostack), options(nostack)); + | ^^^^^^^ + +error: aborting due to 6 previous errors