clif-util: try both global and target-dependent settings when parsing --set flags;

cranelift/reader/src/isaspec.rs

@@ -6,7 +6,7 @@
 //! If a test case file contains `isa` commands, the tests will only be run against the specified
 //! ISAs. If the file contains no `isa` commands, the tests will be run against all supported ISAs.
 
-use crate::error::{Location, ParseResult};
+use crate::error::{Location, ParseError};
 use crate::testcommand::TestOption;
 use cranelift_codegen::isa::TargetIsa;
 use cranelift_codegen::settings::{Configurable, Flags, SetError};
@@ -34,12 +34,49 @@ impl IsaSpec {
     }
 }
 
+/// An error type returned by `parse_options`.
+pub enum ParseOptionError {
+    /// A generic ParseError.
+    Generic(ParseError),
+
+    /// An unknown flag was used, with the given name at the given location.
+    UnknownFlag {
+        /// Location where the flag was given.
+        loc: Location,
+        /// Name of the unknown flag.
+        name: String,
+    },
+}
+
+impl From<ParseOptionError> for ParseError {
+    fn from(err: ParseOptionError) -> Self {
+        match err {
+            ParseOptionError::Generic(err) => err,
+            ParseOptionError::UnknownFlag { loc, name } => Self {
+                location: loc,
+                message: format!("unknown setting '{}'", name),
+                is_warning: false,
+            },
+        }
+    }
+}
+
+macro_rules! option_err {
+    ( $loc:expr, $fmt:expr, $( $arg:expr ),+ ) => {
+        Err($crate::ParseOptionError::Generic($crate::ParseError {
+            location: $loc.clone(),
+            message: format!( $fmt, $( $arg ),+ ),
+            is_warning: false,
+        }))
+    };
+}
+
 /// Parse an iterator of command line options and apply them to `config`.
 pub fn parse_options<'a, I>(
     iter: I,
     config: &mut dyn Configurable,
     loc: Location,
-) -> ParseResult<()>
+) -> Result<(), ParseOptionError>
 where
     I: Iterator<Item = &'a str>,
 {
@@ -47,15 +84,21 @@ where
         match opt {
             TestOption::Flag(name) => match config.enable(name) {
                 Ok(_) => {}
-                Err(SetError::BadName(name)) => return err!(loc, "unknown flag '{}'", name),
+                Err(SetError::BadName(name)) => {
-                Err(_) => return err!(loc, "not a boolean flag: '{}'", opt),
+                    return Err(ParseOptionError::UnknownFlag { loc, name })
+                }
+                Err(_) => return option_err!(loc, "not a boolean flag: '{}'", opt),
             },
             TestOption::Value(name, value) => match config.set(name, value) {
                 Ok(_) => {}
-                Err(SetError::BadName(name)) => return err!(loc, "unknown setting '{}'", name),
+                Err(SetError::BadName(name)) => {
-                Err(SetError::BadType) => return err!(loc, "invalid setting type: '{}'", opt),
+                    return Err(ParseOptionError::UnknownFlag { loc, name })
+                }
+                Err(SetError::BadType) => {
+                    return option_err!(loc, "invalid setting type: '{}'", opt)
+                }
                 Err(SetError::BadValue(expected)) => {
-                    return err!(
+                    return option_err!(
                         loc,
                         "invalid setting value for '{}', expected {}",
                         opt,

cranelift/reader/src/lib.rs

@@ -27,7 +27,7 @@
 )]
 
 pub use crate::error::{Location, ParseError, ParseResult};
-pub use crate::isaspec::{parse_options, IsaSpec};
+pub use crate::isaspec::{parse_options, IsaSpec, ParseOptionError};
 pub use crate::parser::{parse_functions, parse_run_command, parse_test, ParseOptions};
 pub use crate::run_command::{Comparison, DataValue, Invocation, RunCommand};
 pub use crate::sourcemap::SourceMap;

cranelift/reader/src/parser.rs

@@ -1178,7 +1178,8 @@ impl<'a> Parser<'a> {
                         self.consume_line().trim().split_whitespace(),
                         &mut flag_builder,
                         self.loc,
-                    )?;
+                    )
+                    .map_err(|err| ParseError::from(err))?;
                 }
                 "target" => {
                     let loc = self.loc;

cranelift/src/utils.rs

@@ -3,7 +3,7 @@
 use cranelift_codegen::isa;
 use cranelift_codegen::isa::TargetIsa;
 use cranelift_codegen::settings::{self, FlagsOrIsa};
-use cranelift_reader::{parse_options, Location};
+use cranelift_reader::{parse_options, Location, ParseError, ParseOptionError};
 use std::fs::File;
 use std::io::{self, Read};
 use std::path::Path;
@@ -46,12 +46,21 @@ pub fn parse_sets_and_triple(
     flag_triple: &str,
 ) -> Result<OwnedFlagsOrIsa, String> {
     let mut flag_builder = settings::builder();
-    parse_options(
+
+    // Collect unknown system-wide settings, so we can try to parse them as target specific
+    // settings, if a target is defined.
+    let mut unknown_settings = Vec::new();
+    match parse_options(
         flag_set.iter().map(|x| x.as_str()),
         &mut flag_builder,
         Location { line_number: 0 },
-    )
+    ) {
-    .map_err(|err| err.to_string())?;
+        Err(ParseOptionError::UnknownFlag { name, .. }) => {
+            unknown_settings.push(name);
+        }
+        Err(ParseOptionError::Generic(err)) => return Err(err.to_string()),
+        Ok(()) => {}
+    }
 
     let mut words = flag_triple.trim().split_whitespace();
     // Look for `target foo`.
@@ -60,6 +69,7 @@ pub fn parse_sets_and_triple(
             Ok(triple) => triple,
             Err(parse_error) => return Err(parse_error.to_string()),
         };
+
         let mut isa_builder = isa::lookup(triple).map_err(|err| match err {
             isa::LookupError::SupportDisabled => {
                 format!("support for triple '{}' is disabled", triple_name)
@@ -69,14 +79,29 @@ pub fn parse_sets_and_triple(
                 triple_name
             ),
         })?;
+
+        // Try to parse system-wide unknown settings as target-specific settings.
+        parse_options(
+            unknown_settings.iter().map(|x| x.as_str()),
+            &mut isa_builder,
+            Location { line_number: 0 },
+        )
+        .map_err(|err| ParseError::from(err).to_string())?;
+
         // Apply the ISA-specific settings to `isa_builder`.
         parse_options(words, &mut isa_builder, Location { line_number: 0 })
-            .map_err(|err| err.to_string())?;
+            .map_err(|err| ParseError::from(err).to_string())?;
 
         Ok(OwnedFlagsOrIsa::Isa(
             isa_builder.finish(settings::Flags::new(flag_builder)),
         ))
     } else {
+        if !unknown_settings.is_empty() {
+            return Err(format!(
+                "unknown settings: '{}'",
+                unknown_settings.join("', '")
+            ));
+        }
         Ok(OwnedFlagsOrIsa::Flags(settings::Flags::new(flag_builder)))
     }
 }