ZJIT: Ensure clippy passes and silence unnecessary warnings (ruby#14439)

Author: aidenfoxivey

.github/workflows/zjit-ubuntu.yml

@@ -27,6 +27,25 @@ permissions:
   contents: read
 
 jobs:
+  lint:
+    name: cargo clippy
+
+    runs-on: ubuntu-22.04
+
+    if: >-
+      ${{!(false
+      || contains(github.event.head_commit.message, '[DOC]')
+      || contains(github.event.pull_request.title, '[DOC]')
+      || contains(github.event.pull_request.labels.*.name, 'Documentation')
+      || (github.event_name == 'push' && github.event.pull_request.user.login == 'dependabot[bot]')
+      )}}
+
+    steps:
+      - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
+
+      - run: cargo clippy --all-targets --all-features
+        working-directory: zjit
+
   make:
     strategy:
       fail-fast: false

zjit/src/asm/arm64/inst/branch.rs

@@ -89,7 +89,7 @@ mod tests {
     #[test]
     fn test_ret() {
         let result: u32 = Branch::ret(30).into();
-        assert_eq!(0xd65f03C0, result);
+        assert_eq!(0xd65f03c0, result);
     }
 
     #[test]

zjit/src/asm/arm64/inst/load_literal.rs

@@ -1,3 +1,5 @@
+#![allow(clippy::identity_op)]
+
 use super::super::arg::{InstructionOffset, truncate_imm};
 
 /// The size of the operands being operated on.

zjit/src/asm/arm64/inst/mov.rs

@@ -145,14 +145,14 @@ mod tests {
     fn test_movk_shifted_16() {
         let inst = Mov::movk(0, 123, 16, 64);
         let result: u32 = inst.into();
-        assert_eq!(0xf2A00f60, result);
+        assert_eq!(0xf2a00f60, result);
     }
 
     #[test]
     fn test_movk_shifted_32() {
         let inst = Mov::movk(0, 123, 32, 64);
         let result: u32 = inst.into();
-        assert_eq!(0xf2C00f60, result);
+        assert_eq!(0xf2c00f60, result);
     }
 
     #[test]

zjit/src/asm/arm64/mod.rs

@@ -1,4 +1,8 @@
 #![allow(dead_code)] // For instructions and operands we're not currently using.
+#![allow(clippy::upper_case_acronyms)]
+#![allow(clippy::identity_op)]
+#![allow(clippy::self_named_constructors)]
+#![allow(clippy::unusual_byte_groupings)]
 
 use crate::asm::CodeBlock;
 
@@ -1590,7 +1594,7 @@ mod tests {
 
     #[test]
     fn test_nop() {
-        check_bytes("1f2003d5", |cb| nop(cb));
+        check_bytes("1f2003d5", nop);
     }
 
     #[test]

zjit/src/asm/arm64/opnd.rs

@@ -79,10 +79,7 @@ impl A64Opnd {
 
     /// Convenience function to check if this operand is a register.
     pub fn is_reg(&self) -> bool {
-        match self {
-            A64Opnd::Reg(_) => true,
-            _ => false
-        }
+        matches!(self, A64Opnd::Reg(_))
     }
 
     /// Unwrap a register from an operand.

zjit/src/asm/x86_64/mod.rs

@@ -163,10 +163,7 @@ impl X86Opnd {
     }
 
     pub fn is_some(&self) -> bool {
-        match self {
-            X86Opnd::None => false,
-            _ => true
-        }
+        !matches!(self, X86Opnd::None)
     }
 
 }
@@ -284,11 +281,11 @@ pub fn mem_opnd(num_bits: u8, base_reg: X86Opnd, disp: i32) -> X86Opnd
     } else {
         X86Opnd::Mem(
             X86Mem {
-                num_bits: num_bits,
+                num_bits,
                 base_reg_no: base_reg.reg_no,
                 idx_reg_no: None,
                 scale_exp: 0,
-                disp: disp,
+                disp,
             }
         )
     }

zjit/src/asm/x86_64/tests.rs

@@ -33,7 +33,7 @@ fn test_add() {
 fn test_add_unsigned() {
     // ADD r/m8, imm8
     check_bytes("4180c001", |cb| add(cb, R8B, uimm_opnd(1)));
-    check_bytes("4180c07f", |cb| add(cb, R8B, imm_opnd(i8::MAX.try_into().unwrap())));
+    check_bytes("4180c07f", |cb| add(cb, R8B, imm_opnd(i8::MAX.into())));
 
     // ADD r/m16, imm16
     check_bytes("664183c001", |cb| add(cb, R8W, uimm_opnd(1)));
@@ -102,7 +102,7 @@ fn test_cmp() {
 
 #[test]
 fn test_cqo() {
-    check_bytes("4899", |cb| cqo(cb));
+    check_bytes("4899", cqo);
 }
 
 #[test]
@@ -341,7 +341,7 @@ fn test_push() {
 
 #[test]
 fn test_ret() {
-    check_bytes("c3", |cb| ret(cb));
+    check_bytes("c3", ret);
 }
 
 #[test]

zjit/src/backend/arm64/mod.rs

@@ -183,7 +183,7 @@ fn emit_load_value(cb: &mut CodeBlock, rd: A64Opnd, value: u64) -> usize {
 /// List of registers that can be used for register allocation.
 /// This has the same number of registers for x86_64 and arm64.
 /// SCRATCH0 and SCRATCH1 are excluded.
-pub const ALLOC_REGS: &'static [Reg] = &[
+pub const ALLOC_REGS: &[Reg] = &[
     X0_REG,
     X1_REG,
     X2_REG,
@@ -386,15 +386,12 @@ impl Assembler
             let mut opnd_iter = insn.opnd_iter_mut();
 
             while let Some(opnd) = opnd_iter.next() {
-                match opnd {
-                    Opnd::Value(value) => {
-                        if value.special_const_p() {
-                            *opnd = Opnd::UImm(value.as_u64());
-                        } else if !is_load {
-                            *opnd = asm.load(*opnd);
-                        }
-                    },
-                    _ => {}
+                if let Opnd::Value(value) = opnd {
+                    if value.special_const_p() {
+                        *opnd = Opnd::UImm(value.as_u64());
+                    } else if !is_load {
+                        *opnd = asm.load(*opnd);
+                    }
                 };
             }
 
@@ -481,7 +478,7 @@ impl Assembler
                     // which is both the return value and first argument register
                     if !opnds.is_empty() {
                         let mut args: Vec<(Reg, Opnd)> = vec![];
-                        for (idx, opnd) in opnds.into_iter().enumerate().rev() {
+                        for (idx, opnd) in opnds.iter_mut().enumerate().rev() {
                             // If the value that we're sending is 0, then we can use
                             // the zero register, so in this case we'll just send
                             // a UImm of 0 along as the argument to the move.
@@ -1411,7 +1408,7 @@ impl Assembler
 ///
 /// If a, b, and c are all registers.
 fn merge_three_reg_mov(
-    live_ranges: &Vec<LiveRange>,
+    live_ranges: &[LiveRange],
     iterator: &mut std::iter::Peekable<impl Iterator<Item = (usize, Insn)>>,
     left: &Opnd,
     right: &Opnd,
@@ -1566,7 +1563,7 @@ mod tests {
 
     #[test]
     fn frame_setup_and_teardown() {
-        const THREE_REGS: &'static [Opnd] = &[Opnd::Reg(X19_REG), Opnd::Reg(X20_REG), Opnd::Reg(X21_REG)];
+        const THREE_REGS: &[Opnd] = &[Opnd::Reg(X19_REG), Opnd::Reg(X20_REG), Opnd::Reg(X21_REG)];
         // Test 3 preserved regs (odd), odd slot_count
         {
             let (mut asm, mut cb) = setup_asm();
@@ -1607,7 +1604,7 @@ mod tests {
 
         // Test 4 preserved regs (even), odd slot_count
         {
-            static FOUR_REGS: &'static [Opnd] = &[Opnd::Reg(X19_REG), Opnd::Reg(X20_REG), Opnd::Reg(X21_REG), Opnd::Reg(X22_REG)];
+            static FOUR_REGS: &[Opnd] = &[Opnd::Reg(X19_REG), Opnd::Reg(X20_REG), Opnd::Reg(X21_REG), Opnd::Reg(X22_REG)];
             let (mut asm, mut cb) = setup_asm();
             asm.frame_setup(FOUR_REGS, 3);
             asm.frame_teardown(FOUR_REGS);

zjit/src/backend/lir.rs

@@ -18,7 +18,7 @@ pub use crate::backend::current::{
 };
 pub const SCRATCH_OPND: Opnd = Opnd::Reg(Assembler::SCRATCH_REG);
 
-pub static JIT_PRESERVED_REGS: &'static [Opnd] = &[CFP, SP, EC];
+pub static JIT_PRESERVED_REGS: &[Opnd] = &[CFP, SP, EC];
 
 // Memory operand base
 #[derive(Clone, Copy, PartialEq, Eq, Debug)]
@@ -99,17 +99,17 @@ impl Opnd
                 assert!(base_reg.num_bits == 64);
                 Opnd::Mem(Mem {
                     base: MemBase::Reg(base_reg.reg_no),
-                    disp: disp,
-                    num_bits: num_bits,
+                    disp,
+                    num_bits,
                 })
             },
 
             Opnd::VReg{idx, num_bits: out_num_bits } => {
                 assert!(num_bits <= out_num_bits);
                 Opnd::Mem(Mem {
                     base: MemBase::VReg(idx),
-                    disp: disp,
-                    num_bits: num_bits,
+                    disp,
+                    num_bits,
                 })
             },
 
@@ -1215,8 +1215,8 @@ impl Assembler
         }
 
         // If we find any VReg from previous instructions, extend the live range to insn_idx
-        let mut opnd_iter = insn.opnd_iter();
-        while let Some(opnd) = opnd_iter.next() {
+        let opnd_iter = insn.opnd_iter();
+        for opnd in opnd_iter {
             match *opnd {
                 Opnd::VReg { idx, .. } |
                 Opnd::Mem(Mem { base: MemBase::VReg(idx), .. }) => {
@@ -1243,16 +1243,16 @@ impl Assembler
 
     // Shuffle register moves, sometimes adding extra moves using SCRATCH_REG,
     // so that they will not rewrite each other before they are used.
-    pub fn resolve_parallel_moves(old_moves: &Vec<(Reg, Opnd)>) -> Vec<(Reg, Opnd)> {
+    pub fn resolve_parallel_moves(old_moves: &[(Reg, Opnd)]) -> Vec<(Reg, Opnd)> {
         // Return the index of a move whose destination is not used as a source if any.
-        fn find_safe_move(moves: &Vec<(Reg, Opnd)>) -> Option<usize> {
+        fn find_safe_move(moves: &[(Reg, Opnd)]) -> Option<usize> {
             moves.iter().enumerate().find(|&(_, &(dest_reg, _))| {
                 moves.iter().all(|&(_, src_opnd)| src_opnd != Opnd::Reg(dest_reg))
             }).map(|(index, _)| index)
         }
 
         // Remove moves whose source and destination are the same
-        let mut old_moves: Vec<(Reg, Opnd)> = old_moves.clone().into_iter()
+        let mut old_moves: Vec<(Reg, Opnd)> = old_moves.iter().copied()
             .filter(|&(reg, opnd)| Opnd::Reg(reg) != opnd).collect();
 
         let mut new_moves = vec![];
@@ -1386,19 +1386,19 @@ impl Assembler
                 Some(Opnd::VReg { idx, .. }) => Some(*idx),
                 _ => None,
             };
-            if vreg_idx.is_some() {
-                if live_ranges[vreg_idx.unwrap()].end() == index {
-                    debug!("Allocating a register for VReg({}) at instruction index {} even though it does not live past this index", vreg_idx.unwrap(), index);
+            if let Some(vreg_idx) = vreg_idx {
+                if live_ranges[vreg_idx].end() == index {
+                    debug!("Allocating a register for VReg({}) at instruction index {} even though it does not live past this index", vreg_idx, index);
                 }
                 // This is going to be the output operand that we will set on the
                 // instruction. CCall and LiveReg need to use a specific register.
                 let mut out_reg = match insn {
                     Insn::CCall { .. } => {
-                        Some(pool.take_reg(&C_RET_REG, vreg_idx.unwrap()))
+                        Some(pool.take_reg(&C_RET_REG, vreg_idx))
                     }
                     Insn::LiveReg { opnd, .. } => {
                         let reg = opnd.unwrap_reg();
-                        Some(pool.take_reg(&reg, vreg_idx.unwrap()))
+                        Some(pool.take_reg(&reg, vreg_idx))
                     }
                     _ => None
                 };
@@ -1414,7 +1414,7 @@ impl Assembler
                     if let Some(Opnd::VReg{ idx, .. }) = opnd_iter.next() {
                         if live_ranges[*idx].end() == index {
                             if let Some(reg) = reg_mapping[*idx] {
-                                out_reg = Some(pool.take_reg(&reg, vreg_idx.unwrap()));
+                                out_reg = Some(pool.take_reg(&reg, vreg_idx));
                             }
                         }
                     }
@@ -1423,21 +1423,19 @@ impl Assembler
                 // Allocate a new register for this instruction if one is not
                 // already allocated.
                 if out_reg.is_none() {
-                    out_reg = match &insn {
-                        _ => match pool.alloc_reg(vreg_idx.unwrap()) {
-                            Some(reg) => Some(reg),
-                            None => {
-                                if get_option!(debug) {
-                                    let mut insns = asm.insns;
+                    out_reg = match pool.alloc_reg(vreg_idx) {
+                        Some(reg) => Some(reg),
+                        None => {
+                            if get_option!(debug) {
+                                let mut insns = asm.insns;
+                                insns.push(insn);
+                                for (_, insn) in iterator.by_ref() {
                                     insns.push(insn);
-                                    while let Some((_, insn)) = iterator.next() {
-                                        insns.push(insn);
-                                    }
-                                    dump_live_regs(insns, live_ranges, regs.len(), index);
                                 }
-                                debug!("Register spill not supported");
-                                return Err(CompileError::RegisterSpillOnAlloc);
+                                dump_live_regs(insns, live_ranges, regs.len(), index);
                             }
+                            debug!("Register spill not supported");
+                            return Err(CompileError::RegisterSpillOnAlloc);
                         }
                     };
                 }
@@ -1510,7 +1508,7 @@ impl Assembler
             if is_ccall {
                 // On x86_64, maintain 16-byte stack alignment
                 if cfg!(target_arch = "x86_64") && saved_regs.len() % 2 == 1 {
-                    asm.cpop_into(Opnd::Reg(saved_regs.last().unwrap().0.clone()));
+                    asm.cpop_into(Opnd::Reg(saved_regs.last().unwrap().0));
                 }
                 // Restore saved registers
                 for &(reg, vreg_idx) in saved_regs.iter().rev() {
@@ -1527,7 +1525,6 @@ impl Assembler
 
     /// Compile the instructions down to machine code.
     /// Can fail due to lack of code memory and inopportune code placement, among other reasons.
-    #[must_use]
     pub fn compile(self, cb: &mut CodeBlock) -> Result<(CodePtr, Vec<CodePtr>), CompileError> {
         #[cfg(feature = "disasm")]
         let start_addr = cb.get_write_ptr();
@@ -2038,7 +2035,7 @@ mod tests {
         assert!(matches!(opnd_iter.next(), Some(Opnd::None)));
         assert!(matches!(opnd_iter.next(), Some(Opnd::None)));
 
-        assert!(matches!(opnd_iter.next(), None));
+        assert!(opnd_iter.next().is_none());
     }
 
     #[test]
@@ -2049,7 +2046,7 @@ mod tests {
         assert!(matches!(opnd_iter.next(), Some(Opnd::None)));
         assert!(matches!(opnd_iter.next(), Some(Opnd::None)));
 
-        assert!(matches!(opnd_iter.next(), None));
+        assert!(opnd_iter.next().is_none());
     }
 
     #[test]