Make SettingsError easier to diagnose;

This commit is contained in:
Benjamin Bouvier
2018-10-03 14:58:07 +02:00
committed by Dan Gohman
parent 8b296e4874
commit de1d82b4ba
2 changed files with 38 additions and 17 deletions

View File

@@ -85,7 +85,7 @@ impl Builder {
/// Look up a descriptor by name. /// Look up a descriptor by name.
fn lookup(&self, name: &str) -> SetResult<(usize, detail::Detail)> { fn lookup(&self, name: &str) -> SetResult<(usize, detail::Detail)> {
match probe(self.template, name, simple_hash(name)) { match probe(self.template, name, simple_hash(name)) {
Err(_) => Err(SetError::BadName), Err(_) => Err(SetError::BadName(name.to_string())),
Ok(entry) => { Ok(entry) => {
let d = &self.template.descriptors[self.template.hash_table[entry] as usize]; let d = &self.template.descriptors[self.template.hash_table[entry] as usize];
Ok((d.offset as usize, d.detail)) Ok((d.offset as usize, d.detail))
@@ -98,14 +98,17 @@ fn parse_bool_value(value: &str) -> SetResult<bool> {
match value { match value {
"true" | "on" | "yes" | "1" => Ok(true), "true" | "on" | "yes" | "1" => Ok(true),
"false" | "off" | "no" | "0" => Ok(false), "false" | "off" | "no" | "0" => Ok(false),
_ => Err(SetError::BadValue), _ => Err(SetError::BadValue("bool".to_string())),
} }
} }
fn parse_enum_value(value: &str, choices: &[&str]) -> SetResult<u8> { fn parse_enum_value(value: &str, choices: &[&str]) -> SetResult<u8> {
match choices.iter().position(|&tag| tag == value) { match choices.iter().position(|&tag| tag == value) {
Some(idx) => Ok(idx as u8), Some(idx) => Ok(idx as u8),
None => Err(SetError::BadValue), None => Err(SetError::BadValue(format!(
"any among {}",
choices.join(", ")
))),
} }
} }
@@ -134,13 +137,15 @@ impl Configurable for Builder {
self.set_bit(offset, bit, parse_bool_value(value)?); self.set_bit(offset, bit, parse_bool_value(value)?);
} }
Detail::Num => { Detail::Num => {
self.bytes[offset] = value.parse().map_err(|_| SetError::BadValue)?; self.bytes[offset] = value
.parse()
.map_err(|_| SetError::BadValue("number".to_string()))?;
} }
Detail::Enum { last, enumerators } => { Detail::Enum { last, enumerators } => {
self.bytes[offset] = self.bytes[offset] =
parse_enum_value(value, self.template.enums(last, enumerators))?; parse_enum_value(value, self.template.enums(last, enumerators))?;
} }
Detail::Preset => return Err(SetError::BadName), Detail::Preset => return Err(SetError::BadName(name.to_string())),
} }
Ok(()) Ok(())
} }
@@ -150,16 +155,16 @@ impl Configurable for Builder {
#[derive(Fail, Debug, PartialEq, Eq)] #[derive(Fail, Debug, PartialEq, Eq)]
pub enum SetError { pub enum SetError {
/// No setting by this name exists. /// No setting by this name exists.
#[fail(display = "No existing setting with this name")] #[fail(display = "No existing setting named '{}'", _0)]
BadName, BadName(String),
/// Type mismatch for setting (e.g., setting an enum setting as a bool). /// Type mismatch for setting (e.g., setting an enum setting as a bool).
#[fail(display = "Trying to set a setting with the wrong type")] #[fail(display = "Trying to set a setting with the wrong type")]
BadType, BadType,
/// This is not a valid value for this setting. /// This is not a valid value for this setting.
#[fail(display = "Unexpected value for a setting")] #[fail(display = "Unexpected value for a setting, expected {}", _0)]
BadValue, BadValue(String),
} }
/// A result returned when changing a setting. /// A result returned when changing a setting.
@@ -385,7 +390,7 @@ mod tests {
#[test] #[test]
fn modify_bool() { fn modify_bool() {
let mut b = builder(); let mut b = builder();
assert_eq!(b.enable("not_there"), Err(BadName)); assert_eq!(b.enable("not_there"), Err(BadName("not_there".to_string())));
assert_eq!(b.enable("enable_simd"), Ok(())); assert_eq!(b.enable("enable_simd"), Ok(()));
assert_eq!(b.set("enable_simd", "false"), Ok(())); assert_eq!(b.set("enable_simd", "false"), Ok(()));
@@ -396,10 +401,19 @@ mod tests {
#[test] #[test]
fn modify_string() { fn modify_string() {
let mut b = builder(); let mut b = builder();
assert_eq!(b.set("not_there", "true"), Err(BadName)); assert_eq!(
assert_eq!(b.set("enable_simd", ""), Err(BadValue)); b.set("not_there", "true"),
assert_eq!(b.set("enable_simd", "best"), Err(BadValue)); Err(BadName("not_there".to_string()))
assert_eq!(b.set("opt_level", "true"), Err(BadValue)); );
assert_eq!(b.set("enable_simd", ""), Err(BadValue("bool".to_string())));
assert_eq!(
b.set("enable_simd", "best"),
Err(BadValue("bool".to_string()))
);
assert_eq!(
b.set("opt_level", "true"),
Err(BadValue("any among default, best, fastest".to_string()))
);
assert_eq!(b.set("opt_level", "best"), Ok(())); assert_eq!(b.set("opt_level", "best"), Ok(()));
assert_eq!(b.set("enable_simd", "0"), Ok(())); assert_eq!(b.set("enable_simd", "0"), Ok(()));

View File

@@ -43,14 +43,21 @@ where
match opt { match opt {
TestOption::Flag(name) => match config.enable(name) { TestOption::Flag(name) => match config.enable(name) {
Ok(_) => {} Ok(_) => {}
Err(SetError::BadName) => return err!(loc, "unknown flag '{}'", opt), Err(SetError::BadName(name)) => return err!(loc, "unknown flag '{}'", name),
Err(_) => return err!(loc, "not a boolean flag: '{}'", opt), Err(_) => return err!(loc, "not a boolean flag: '{}'", opt),
}, },
TestOption::Value(name, value) => match config.set(name, value) { TestOption::Value(name, value) => match config.set(name, value) {
Ok(_) => {} Ok(_) => {}
Err(SetError::BadName) => return err!(loc, "unknown setting '{}'", opt), Err(SetError::BadName(name)) => return err!(loc, "unknown setting '{}'", name),
Err(SetError::BadType) => return err!(loc, "invalid setting type: '{}'", opt), Err(SetError::BadType) => return err!(loc, "invalid setting type: '{}'", opt),
Err(SetError::BadValue) => return err!(loc, "invalid setting value: '{}'", opt), Err(SetError::BadValue(expected)) => {
return err!(
loc,
"invalid setting value for '{}', expected {}",
opt,
expected
)
}
}, },
} }
} }