Merge pull request #429 from Kmeakin/clippy-fix

wezm · web-flow · commit 139340494d86 · 2022-12-13T11:43:12.000+11:00
Apply clippy suggestions
diff --git a/fathom/Cargo.toml b/fathom/Cargo.toml
@@ -4,7 +4,7 @@ version = "0.1.0"
 authors = ["YesLogic Pty. Ltd. <info@yeslogic.com>"]
 repository = "https://github.com/yeslogic/fathom"
 edition = "2021"
-rust-version = "1.56.0"
+rust-version = "1.62.0"
 publish = false
 
 description = "A language for declaratively specifying binary data formats"
diff --git a/fathom/src/core.rs b/fathom/src/core.rs
@@ -555,7 +555,7 @@ pub enum UIntStyle {
 }
 
 /// Constants
-#[derive(Debug, Copy, Clone, PartialOrd)]
+#[derive(Debug, Copy, Clone)]
 pub enum Const {
     Bool(bool),
     U8(u8, UIntStyle),
@@ -566,7 +566,6 @@ pub enum Const {
     S16(i16),
     S32(i32),
     S64(i64),
-    // TODO: use logical equality for floating point numbers
     F32(f32),
     F64(f64),
     Pos(usize),
@@ -585,15 +584,66 @@ impl PartialEq for Const {
             (Const::S16(a), Const::S16(b)) => a == b,
             (Const::S32(a), Const::S32(b)) => a == b,
             (Const::S64(a), Const::S64(b)) => a == b,
-            (Const::F32(a), Const::F32(b)) => a == b,
-            (Const::F64(a), Const::F64(b)) => a == b,
+            (Const::F32(a), Const::F32(b)) => a.total_cmp(&b).is_eq(),
+            (Const::F64(a), Const::F64(b)) => a.total_cmp(&b).is_eq(),
             (Const::Pos(a), Const::Pos(b)) => a == b,
             (Const::Ref(a), Const::Ref(b)) => a == b,
             _ => false,
         }
     }
 }
 
+impl Eq for Const {}
+
+impl PartialOrd for Const {
+    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl Ord for Const {
+    fn cmp(&self, other: &Self) -> std::cmp::Ordering {
+        match (*self, *other) {
+            (Const::Bool(a), Const::Bool(b)) => a.cmp(&b),
+            (Const::U8(a, _), Const::U8(b, _)) => a.cmp(&b),
+            (Const::U16(a, _), Const::U16(b, _)) => a.cmp(&b),
+            (Const::U32(a, _), Const::U32(b, _)) => a.cmp(&b),
+            (Const::U64(a, _), Const::U64(b, _)) => a.cmp(&b),
+            (Const::S8(a), Const::S8(b)) => a.cmp(&b),
+            (Const::S16(a), Const::S16(b)) => a.cmp(&b),
+            (Const::S32(a), Const::S32(b)) => a.cmp(&b),
+            (Const::S64(a), Const::S64(b)) => a.cmp(&b),
+            (Const::F32(a), Const::F32(b)) => a.total_cmp(&b),
+            (Const::F64(a), Const::F64(b)) => a.total_cmp(&b),
+            (Const::Pos(a), Const::Pos(b)) => a.cmp(&b),
+            (Const::Ref(a), Const::Ref(b)) => a.cmp(&b),
+            _ => {
+                fn discriminant(r#const: &Const) -> usize {
+                    match r#const {
+                        Const::Bool(_) => 0,
+                        Const::U8(_, _) => 1,
+                        Const::U16(_, _) => 2,
+                        Const::U32(_, _) => 3,
+                        Const::U64(_, _) => 4,
+                        Const::S8(_) => 5,
+                        Const::S16(_) => 6,
+                        Const::S32(_) => 7,
+                        Const::S64(_) => 8,
+                        Const::F32(_) => 9,
+                        Const::F64(_) => 10,
+                        Const::Pos(_) => 11,
+                        Const::Ref(_) => 12,
+                    }
+                }
+
+                let tag1 = discriminant(self);
+                let tag2 = discriminant(other);
+                tag1.cmp(&tag2)
+            }
+        }
+    }
+}
+
 pub trait ToBeBytes<const N: usize> {
     fn to_be_bytes(self) -> [u8; N];
 }
@@ -622,16 +672,16 @@ pub trait UIntStyled<const N: usize>:
 impl UIntStyle {
     pub fn format<T: UIntStyled<N>, const N: usize>(&self, number: T) -> String {
         match self {
-            UIntStyle::Binary => format!("0b{:b}", number),
+            UIntStyle::Binary => format!("0b{number:b}"),
             UIntStyle::Decimal => number.to_string(),
-            UIntStyle::Hexadecimal => format!("0x{:x}", number),
+            UIntStyle::Hexadecimal => format!("0x{number:x}"),
             UIntStyle::Ascii => {
                 let bytes = number.to_be_bytes();
                 if bytes.iter().all(|c| c.is_ascii() && !c.is_ascii_control()) {
                     let s = std::str::from_utf8(&bytes).unwrap(); // unwrap safe due to above check
-                    format!("\"{}\"", s)
+                    format!("\"{s}\"")
                 } else {
-                    format!("0x{:x}", number)
+                    format!("0x{number:x}")
                 }
             }
         }
diff --git a/fathom/src/core/binary.rs b/fathom/src/core/binary.rs
@@ -584,7 +584,7 @@ impl<'arena, 'data> Context<'arena, 'data> {
         // especially without succumbing to non-termination, but we'll panic
         // here just in case.
         if self.lookup_ref(pos, format).is_some() {
-            panic!("recursion found when storing cached reference {}", pos);
+            panic!("recursion found when storing cached reference {pos}");
         }
 
         // Store the parsed reference in the reference cache
diff --git a/fathom/src/driver.rs b/fathom/src/driver.rs
@@ -93,13 +93,13 @@ impl<'surface, 'core> Driver<'surface, 'core> {
             };
 
             let diagnostic = Diagnostic::bug()
-                .with_message(format!("compiler panicked at '{}'", message))
+                .with_message(format!("compiler panicked at '{message}'"))
                 .with_notes(vec![
                     match location {
-                        Some(location) => format!("panicked at: {}", location),
+                        Some(location) => format!("panicked at: {location}"),
                         None => "panicked at: unknown location".to_owned(),
                     },
-                    format!("please file a bug report at: {}", BUG_REPORT_URL),
+                    format!("please file a bug report at: {BUG_REPORT_URL}"),
                     // TODO: print rust backtrace
                     // TODO: print fathom backtrace
                 ]);
@@ -403,7 +403,7 @@ impl<'surface, 'core> Driver<'surface, 'core> {
 
     fn emit_read_diagnostic(&self, name: impl std::fmt::Display, error: std::io::Error) {
         let diagnostic =
-            Diagnostic::error().with_message(format!("couldn't read `{}`: {}", name, error));
+            Diagnostic::error().with_message(format!("couldn't read `{name}`: {error}"));
         self.emit_diagnostic(diagnostic);
     }
 
@@ -450,22 +450,20 @@ impl<'surface, 'core> Driver<'surface, 'core> {
                 .with_notes(vec![format!("option_unwrap was called on a none value.")]),
             ReadError::BufferError(span, err) => self.buffer_error_to_diagnostic(err, span),
             ReadError::InvalidFormat(span) | ReadError::InvalidValue(span) => Diagnostic::bug()
-                .with_message(format!("unexpected error '{}'", err))
+                .with_message(format!("unexpected error '{err}'"))
                 .with_labels(
                     IntoIterator::into_iter([label_for_span(&span)])
                         .into_iter()
                         .flatten()
                         .collect(),
                 )
                 .with_notes(vec![format!(
-                    "please file a bug report at: {}",
-                    BUG_REPORT_URL
+                    "please file a bug report at: {BUG_REPORT_URL}"
                 )]),
             ReadError::UnknownItem => Diagnostic::bug()
-                .with_message(format!("unexpected error '{}'", err))
+                .with_message(format!("unexpected error '{err}'"))
                 .with_notes(vec![format!(
-                    "please file a bug report at: {}",
-                    BUG_REPORT_URL
+                    "please file a bug report at: {BUG_REPORT_URL}"
                 )]),
         }
     }
@@ -492,8 +490,7 @@ impl<'surface, 'core> Driver<'surface, 'core> {
                         .collect(),
                 )
                 .with_notes(vec![format!(
-                    "The offset {} is before the start of the buffer.",
-                    offset
+                    "The offset {offset} is before the start of the buffer."
                 )]),
             BufferError::SetOffsetAfterEndOfBuffer {
                 offset: Some(offset),
@@ -506,8 +503,7 @@ impl<'surface, 'core> Driver<'surface, 'core> {
                         .collect(),
                 )
                 .with_notes(vec![format!(
-                    "The offset {} is beyond the end of the buffer.",
-                    offset
+                    "The offset {offset} is beyond the end of the buffer."
                 )]),
             BufferError::SetOffsetAfterEndOfBuffer { offset: None } => Diagnostic::error()
                 .with_message(err.to_string())
@@ -521,16 +517,15 @@ impl<'surface, 'core> Driver<'surface, 'core> {
                     "The offset is beyond the end of the buffer (overflow).",
                 )]),
             BufferError::PositionOverflow => Diagnostic::bug()
-                .with_message(format!("unexpected error '{}'", err))
+                .with_message(format!("unexpected error '{err}'"))
                 .with_labels(
                     IntoIterator::into_iter([label_for_span(&span)])
                         .into_iter()
                         .flatten()
                         .collect(),
                 )
                 .with_notes(vec![format!(
-                    "please file a bug report at: {}",
-                    BUG_REPORT_URL
+                    "please file a bug report at: {BUG_REPORT_URL}"
                 )]),
         }
     }
diff --git a/fathom/src/env.rs b/fathom/src/env.rs
@@ -257,11 +257,11 @@ impl<Entry> SliceEnv<Entry> {
 
 impl<Entry: PartialEq> SliceEnv<Entry> {
     pub fn elem_level(&self, entry: &Entry) -> Option<Level> {
-        Iterator::zip(levels(), self.iter()).find_map(|(var, e)| (entry == e).then(|| var))
+        Iterator::zip(levels(), self.iter()).find_map(|(var, e)| (entry == e).then_some(var))
     }
 
     pub fn elem_index(&self, entry: &Entry) -> Option<Index> {
-        Iterator::zip(indices(), self.iter().rev()).find_map(|(var, e)| (entry == e).then(|| var))
+        Iterator::zip(indices(), self.iter().rev()).find_map(|(var, e)| (entry == e).then_some(var))
     }
 }
 
diff --git a/fathom/src/source.rs b/fathom/src/source.rs
@@ -47,7 +47,7 @@ impl StringInterner {
         if max_index >= len {
             self.tuple_labels.reserve(max_index.saturating_sub(cap));
             for index in len..=max_index {
-                let label = self.get_or_intern(format!("_{}", index));
+                let label = self.get_or_intern(format!("_{index}"));
                 self.tuple_labels.push(label);
             }
         }
diff --git a/fathom/src/surface.rs b/fathom/src/surface.rs
@@ -459,11 +459,11 @@ impl ParseMessage {
                 token,
                 expected,
             } => Diagnostic::error()
-                .with_message(format!("unexpected token {}", token))
+                .with_message(format!("unexpected token {token}"))
                 .with_labels(vec![primary_label(range).with_message("unexpected token")])
                 .with_notes(format_expected(expected).map_or(Vec::new(), |message| vec![message])),
             ParseMessage::ExtraToken { range, token } => Diagnostic::error()
-                .with_message(format!("extra token {}", token))
+                .with_message(format!("extra token {token}"))
                 .with_labels(vec![primary_label(range).with_message("extra token")]),
         }
     }
@@ -479,7 +479,7 @@ fn format_expected(expected: &[impl std::fmt::Display]) -> Option<String> {
     use itertools::Itertools;
 
     expected.split_last().map(|items| match items {
-        (last, []) => format!("expected {}", last),
+        (last, []) => format!("expected {last}"),
         (last, expected) => format!("expected {} or {}", expected.iter().format(", "), last),
     })
 }
diff --git a/fathom/src/surface/elaboration.rs b/fathom/src/surface/elaboration.rs
@@ -457,7 +457,7 @@ impl<'interner, 'arena> Context<'interner, 'arena> {
         }
 
         let filtered_fields = (fields.iter().enumerate())
-            .filter_map(move |(index, field)| (!duplicate_indices.contains(&index)).then(|| field));
+            .filter_map(move |(index, field)| (!duplicate_indices.contains(&index)).then_some(field));
 
         (labels.into(), filtered_fields)
     }
diff --git a/fathom/src/surface/elaboration/reporting.rs b/fathom/src/surface/elaboration/reporting.rs
diff --git a/fathom/tests/source_tests.rs b/fathom/tests/source_tests.rs

Original file line number	Diff line number	Diff line change
`@@ -584,7 +584,7 @@ impl<'arena, 'data> Context<'arena, 'data> {`
`584`	`584`	`// especially without succumbing to non-termination, but we'll panic`
`585`	`585`	`// here just in case.`
`586`	`586`	`if self.lookup_ref(pos, format).is_some() {`
`587`		`- panic!("recursion found when storing cached reference {}", pos);`
	`587`	`+ panic!("recursion found when storing cached reference {pos}");`
`588`	`588`	`}`
`589`	`589`
`590`	`590`	`// Store the parsed reference in the reference cache`
Original file line number	Diff line number	Diff line change
`@@ -257,11 +257,11 @@ impl<Entry> SliceEnv<Entry> {`
`257`	`257`
`258`	`258`	`impl<Entry: PartialEq> SliceEnv<Entry> {`
`259`	`259`	`pub fn elem_level(&self, entry: &Entry) -> Option<Level> {`
`260`		`- Iterator::zip(levels(), self.iter()).find_map(\|(var, e)\| (entry == e).then(\|\| var))`
	`260`	`+ Iterator::zip(levels(), self.iter()).find_map(\|(var, e)\| (entry == e).then_some(var))`
`261`	`261`	`}`
`262`	`262`
`263`	`263`	`pub fn elem_index(&self, entry: &Entry) -> Option<Index> {`
`264`		`- Iterator::zip(indices(), self.iter().rev()).find_map(\|(var, e)\| (entry == e).then(\|\| var))`
	`264`	`+ Iterator::zip(indices(), self.iter().rev()).find_map(\|(var, e)\| (entry == e).then_some(var))`
`265`	`265`	`}`
`266`	`266`	`}`
`267`	`267`
Original file line number	Diff line number	Diff line change
`@@ -47,7 +47,7 @@ impl StringInterner {`
`47`	`47`	`if max_index >= len {`
`48`	`48`	`self.tuple_labels.reserve(max_index.saturating_sub(cap));`
`49`	`49`	`for index in len..=max_index {`
`50`		`- let label = self.get_or_intern(format!("_{}", index));`
	`50`	`+ let label = self.get_or_intern(format!("_{index}"));`
`51`	`51`	`self.tuple_labels.push(label);`
`52`	`52`	`}`
`53`	`53`	`}`
Original file line number	Diff line number	Diff line change
`@@ -457,7 +457,7 @@ impl<'interner, 'arena> Context<'interner, 'arena> {`
`457`	`457`	`}`
`458`	`458`
`459`	`459`	`let filtered_fields = (fields.iter().enumerate())`
`460`		`- .filter_map(move \|(index, field)\| (!duplicate_indices.contains(&index)).then(\|\| field));`
	`460`	`+ .filter_map(move \|(index, field)\| (!duplicate_indices.contains(&index)).then_some(field));`
`461`	`461`
`462`	`462`	`(labels.into(), filtered_fields)`
`463`	`463`	`}`