chore: add clippy lints

Author: andreiltd

.github/workflows/ci.yml

@@ -19,6 +19,8 @@ jobs:
       run: cargo build --verbose
     - name: Run tests
       run: cargo test --verbose
+    - name: Run clippy
+      run: cargo clippy
 
   rustfmt:
     runs-on: ubuntu-latest

Cargo.toml

@@ -25,3 +25,23 @@ bincode = "1.3.3"
 sha2 = "0.10.8"
 sqlite = "0.36.0"
 serde = { version = "1.0.197", features = ["derive"] }
+
+[lints.clippy]
+# The default set of lints in Clippy is viewed as "too noisy" right now so
+# they're all turned off by default. Selective lints are then enabled below as
+# necessary.
+all = { level = 'allow', priority = -1 }
+clone_on_copy = 'warn'
+map_clone = 'warn'
+uninlined_format_args = 'warn'
+unnecessary_to_owned = 'warn'
+manual_strip = 'warn'
+useless_conversion = 'warn'
+unnecessary_mut_passed = 'warn'
+unnecessary_fallible_conversions = 'warn'
+unnecessary_cast = 'warn'
+allow_attributes_without_reason = 'warn'
+from_over_into = 'warn'
+redundant_field_names = 'warn'
+multiple_bound_locations = 'warn'
+extra_unused_type_parameters = 'warn'

src/constant_offsets.rs

@@ -59,7 +59,7 @@ pub fn run(func: &mut FunctionBody, cfg: &CFGInfo) {
     }
 
     while let Some(block) = workqueue.pop_front() {
-        log::trace!("processing {}", block);
+        log::trace!("processing {block}");
         workqueue_set.remove(&block);
 
         for &inst in &func.blocks[block].insts {
@@ -75,7 +75,7 @@ pub fn run(func: &mut FunctionBody, cfg: &CFGInfo) {
 
                 ValueDef::Operator(op, args, tys) if tys.len() == 1 => {
                     let args = &func.arg_pool[*args];
-                    log::trace!(" -> args = {:?}", args);
+                    log::trace!(" -> args = {args:?}");
 
                     match op {
                         Operator::I32Const { value } => {
@@ -231,10 +231,7 @@ pub fn run(func: &mut FunctionBody, cfg: &CFGInfo) {
             offset_base_const.insert(value, k);
             offset_base.insert(value, add);
             log::trace!(
-                "created common base {} (and const {}) associated with offset-from value {}",
-                k,
-                add,
-                value
+                "created common base {k} (and const {add}) associated with offset-from value {value}"
             );
         } else {
             offset_base.insert(value, value);
@@ -244,7 +241,7 @@ pub fn run(func: &mut FunctionBody, cfg: &CFGInfo) {
     // Now, for each value that's an Offset, rewrite it to an add
     // instruction.
     for (block, block_def) in func.blocks.entries_mut() {
-        log::trace!("rewriting in block {}", block);
+        log::trace!("rewriting in block {block}");
         let mut computed_offsets: FxHashMap<AbsValue, Value> = FxHashMap::default();
         let mut new_insts = vec![];
 
@@ -265,13 +262,13 @@ pub fn run(func: &mut FunctionBody, cfg: &CFGInfo) {
                     let args = &func.arg_pool[*args];
                     let tys = *tys;
                     let addr = args[0];
-                    log::trace!("load/store with addr {}", addr);
+                    log::trace!("load/store with addr {addr}");
                     if let AbsValue::Offset(base, this_offset) = values[addr] {
-                        log::trace!("inst {} is a load/store with addr that is offset from base {}; pushing offset into instruction", inst, base);
+                        log::trace!("inst {inst} is a load/store with addr that is offset from base {base}; pushing offset into instruction");
                         // Update the offset embedded in the Operator
                         // and use the `base` value instead as the
                         // address arg.
-                        let mut op = op.clone();
+                        let mut op = *op;
                         let mut args = args.iter().cloned().collect::<Vec<_>>();
                         let common_base = *offset_base.get(&base).unwrap();
                         let offset = *min_offset_from.get(&base).unwrap();
@@ -326,12 +323,12 @@ pub fn run(func: &mut FunctionBody, cfg: &CFGInfo) {
                         let add = func.values.push(ValueDef::Operator(op, args, i32_ty));
                         func.source_locs[k] = func.source_locs[inst];
                         func.source_locs[add] = func.source_locs[inst];
-                        log::trace!(" -> recomputed as {}", add);
+                        log::trace!(" -> recomputed as {add}");
                         new_insts.push(add);
                         add
                     }
                 });
-                log::trace!(" -> rewrite to {}", computed_offset);
+                log::trace!(" -> rewrite to {computed_offset}");
                 func.values[inst] = ValueDef::Alias(computed_offset);
             } else {
                 new_insts.push(inst);

src/dce.rs

@@ -48,7 +48,7 @@ fn scan_block(func: &FunctionBody, block: Block, used: &mut FxHashSet<Value>) ->
         changed
     };
 
-    log::trace!("DCE: scanning {}", block);
+    log::trace!("DCE: scanning {block}");
     let mut changed = false;
 
     func.blocks[block].terminator.visit_targets(|target| {
@@ -57,23 +57,20 @@ fn scan_block(func: &FunctionBody, block: Block, used: &mut FxHashSet<Value>) ->
         for (&arg, &(_, param)) in target.args.iter().zip(succ_params.iter()) {
             if used.contains(&param) {
                 log::trace!(
-                    "  -> succ blockparam {} is used; marking arg {} used from term on {}",
-                    param,
-                    arg,
-                    block,
+                    "  -> succ blockparam {param} is used; marking arg {arg} used from term on {block}",
                 );
                 changed |= mark_used(used, arg);
             }
         }
     });
     match &func.blocks[block].terminator {
         Terminator::CondBr { cond: value, .. } | Terminator::Select { value, .. } => {
-            log::trace!(" -> marking branch input {} used", value);
+            log::trace!(" -> marking branch input {value} used");
             changed |= mark_used(used, *value);
         }
         Terminator::Return { values } => {
             for &value in values {
-                log::trace!(" -> marking return value {} used", value);
+                log::trace!(" -> marking return value {value} used");
                 changed |= mark_used(used, value);
             }
         }
@@ -87,7 +84,7 @@ fn scan_block(func: &FunctionBody, block: Block, used: &mut FxHashSet<Value>) ->
             }
             ValueDef::PickOutput(value, ..) => {
                 if used.contains(&inst) {
-                    log::trace!(" -> marking pick-output src {} used", value);
+                    log::trace!(" -> marking pick-output src {value} used");
                     changed |= mark_used(used, *value);
                 }
             }
@@ -97,7 +94,7 @@ fn scan_block(func: &FunctionBody, block: Block, used: &mut FxHashSet<Value>) ->
                 }
                 if used.contains(&inst) {
                     for &arg in &func.arg_pool[*args] {
-                        log::trace!(" -> marking arg {} used from {}", arg, inst);
+                        log::trace!(" -> marking arg {arg} used from {inst}");
                         changed |= mark_used(used, arg);
                     }
                 }
@@ -118,7 +115,7 @@ pub(crate) fn run(func: &mut FunctionBody, cfg: &CFGInfo) {
     // unreachable block can branch to an unreachable block).
     for (block, block_def) in func.blocks.entries_mut() {
         if cfg.rpo_pos[block].is_none() {
-            log::trace!("removing unreachable block {}", block);
+            log::trace!("removing unreachable block {block}");
             block_def.insts.clear();
             block_def.params.clear();
             block_def.terminator = Terminator::Unreachable;
@@ -135,7 +132,7 @@ pub(crate) fn run(func: &mut FunctionBody, cfg: &CFGInfo) {
         for &block in cfg.rpo.values().rev() {
             changed |= scan_block(func, block, &mut used);
         }
-        log::trace!("done with all blocks; changed = {}", changed);
+        log::trace!("done with all blocks; changed = {changed}");
         if !changed {
             break;
         }

src/directive.rs

@@ -74,7 +74,7 @@ pub(crate) fn collect(module: &Module, im: &mut Image) -> anyhow::Result<Vec<Dir
         }
     };
 
-    log::info!("weval request list head at {:#x}", pending_head_addr);
+    log::info!("weval request list head at {pending_head_addr:#x}");
 
     let heap = match im.main_heap {
         Some(heap) => heap,
@@ -115,7 +115,7 @@ fn decode_weval_req(im: &Image, heap: Memory, head: u32) -> anyhow::Result<Direc
     let func_index_out_addr = im.read_u32(heap, head + 28)?;
     let args = im.read_slice(heap, arg_ptr, arg_len)?.to_vec();
 
-    log::trace!("directive: args {:#x} len {:#x}", arg_ptr, arg_len);
+    log::trace!("directive: args {arg_ptr:#x} len {arg_len:#x}");
 
     Ok(Directive {
         user_id,
@@ -195,7 +195,7 @@ impl DirectiveArgs {
                             16 + padded_len,
                         )
                     }
-                    _ => anyhow::bail!("Invalid type: {}", ty),
+                    _ => anyhow::bail!("Invalid type: {ty}"),
                 }
             } else {
                 (AbstractValue::Runtime(None), None, 16)

src/escape.rs

@@ -28,31 +28,29 @@ fn shadow_stack_escapes(func: &FunctionBody, cfg: &CFGInfo) -> EscapeAnalysisRes
                 | &ValueDef::Operator(Operator::GlobalSet { global_index }, _, _)
                     if global_index.index() == 0 =>
                 {
-                    log::trace!("tainted because global.get/set: {}", inst);
+                    log::trace!("tainted because global.get/set: {inst}");
                     tainted.insert(inst);
                 }
                 &ValueDef::Operator(Operator::I32Add, args, _)
                 | &ValueDef::Operator(Operator::I32Sub, args, _) => {
                     let args = &func.arg_pool[args];
                     if args.iter().any(|arg| tainted.contains(arg)) {
-                        log::trace!("tainted because of arg: {}", inst);
+                        log::trace!("tainted because of arg: {inst}");
                         tainted.insert(inst);
                     }
                 }
                 &ValueDef::Operator(_, args, _) => {
                     let args = &func.arg_pool[args];
                     if args.iter().any(|arg| tainted.contains(arg)) {
-                        log::trace!("shadow stack escape due to inst {}", inst);
+                        log::trace!("shadow stack escape due to inst {inst}");
                         return EscapeAnalysisResult::Escapes;
                     }
                 }
                 &ValueDef::PickOutput(val, _, _) | &ValueDef::Alias(val)
                     if tainted.contains(&val) =>
                 {
                     log::trace!(
-                        "taint on {} propagates to {} because of alias or pick",
-                        val,
-                        inst
+                        "taint on {val} propagates to {inst} because of alias or pick"
                     );
                     tainted.insert(inst);
                 }
@@ -64,8 +62,7 @@ fn shadow_stack_escapes(func: &FunctionBody, cfg: &CFGInfo) -> EscapeAnalysisRes
             &Terminator::CondBr { cond, .. } | &Terminator::Select { value: cond, .. } => {
                 if tainted.contains(&cond) {
                     log::trace!(
-                        "taint on input to conditional branch causes escape: {}",
-                        cond
+                        "taint on input to conditional branch causes escape: {cond}"
                     );
                     return EscapeAnalysisResult::Escapes;
                 }
@@ -111,7 +108,7 @@ fn shadow_stack_escapes(func: &FunctionBody, cfg: &CFGInfo) -> EscapeAnalysisRes
 
 pub(crate) fn remove_shadow_stack_if_non_escaping(func: &mut FunctionBody, cfg: &CFGInfo) {
     if let EscapeAnalysisResult::NonEscaping(values_to_remove) = shadow_stack_escapes(func, &cfg) {
-        log::trace!("removing shadow stack operations: {:?}", values_to_remove);
+        log::trace!("removing shadow stack operations: {values_to_remove:?}");
         let ty_u32 = func.type_pool.single(Type::I32);
         let const_zero = func.values.push(ValueDef::Operator(
             Operator::I32Const { value: 0 },