Use workspace to set clippy linters

use them in yjit and zjit
fix clippy issues

Author: jesse-shopify

Cargo.toml

@@ -49,3 +49,34 @@ overflow-checks = true
 debug = true
 # Use ThinLTO. Much smaller output for a small amount of build time increase.
 lto = "thin"
+
+[workspace.lints.clippy]
+correctness = { level = "forbid", priority = 1 }
+suspicious = { level = "forbid", priority = 1 }
+perf = { level = "forbid", priority = 1 }
+
+complexity = { level = "deny", priority = -1 }
+
+style = { level = "allow", priority = -1 }
+pedantic = { level = "allow", priority = -1 }
+restriction = { level = "allow", priority = -1 }
+
+too_many_arguments = "allow"
+identity_op = "allow"
+
+ptr_arg = "forbid"
+ref_option = "forbid"
+inefficient_to_string = "forbid"
+from_over_into = "forbid"
+unwrap_or_default = "forbid"
+redundant_static_lifetimes = "forbid"
+match_like_matches_macro = "forbid"
+field_reassign_with_default = "forbid"
+inherent_to_string = "forbid"
+owned_cow = "forbid"
+redundant_clone = "forbid"
+str_to_string = "forbid"
+single_char_pattern = "forbid"
+single_char_add_str = "forbid"
+unnecessary_owned_empty_strings = "forbid"
+manual_string_new = "forbid"

yjit/Cargo.toml

@@ -24,3 +24,6 @@ disasm = ["capstone"]
 # from cfg!(debug_assertions) so that we can see disasm of the code
 # that would run in the release mode.
 runtime_checks = []
+
+[lints]
+workspace = true

yjit/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.

yjit/src/asm/mod.rs

@@ -224,16 +224,16 @@ impl CodeBlock {
     }
 
     /// Free the memory pages of given code page indexes
-    fn free_pages(&mut self, page_idxs: &Vec<usize>) {
-        let mut page_idxs = page_idxs.clone();
-        page_idxs.reverse(); // to loop with pop()
+    /// In Rust >= 1.77 this could probably be simplified with chunk_by.
+    fn free_pages(&mut self, page_idxs: &[usize]) {
+        let mut page_idxs = page_idxs.iter().rev().collect::<Vec<_>>();
 
         // Group adjacent page indexes and free them in batches to reduce the # of syscalls.
-        while let Some(page_idx) = page_idxs.pop() {
+        while let Some(&page_idx) = page_idxs.pop() {
             // Group first adjacent page indexes
             let mut batch_idxs = vec![page_idx];
-            while page_idxs.last() == Some(&(batch_idxs.last().unwrap() + 1)) {
-                batch_idxs.push(page_idxs.pop().unwrap());
+            while page_idxs.last() == Some(&&(*batch_idxs.last().unwrap() + 1)) {
+                batch_idxs.push(*page_idxs.pop().unwrap());
             }
 
             // Free the grouped pages at once
@@ -378,7 +378,7 @@ impl CodeBlock {
 
         // Unless this comment is the same as the last one at this same line, add it.
         if this_line_comments.last().map(String::as_str) != Some(comment) {
-            this_line_comments.push(comment.to_string());
+            this_line_comments.push(comment.into());
         }
     }
 
@@ -441,12 +441,12 @@ impl CodeBlock {
 
         // Ignore empty code ranges
         if start_addr == end_addr {
-            return (0..0).into_iter();
+            return 0..0;
         }
 
         let start_page = (start_addr.raw_addr(self) - mem_start) / self.page_size;
         let end_page = (end_addr.raw_addr(self) - mem_start - 1) / self.page_size;
-        (start_page..end_page + 1).into_iter()
+        start_page..end_page + 1
     }
 
     /// Get a (possibly dangling) direct pointer to the current write position

yjit/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)
     }
 
 }

yjit/src/asm/x86_64/tests.rs

@@ -454,7 +454,7 @@ fn block_comments() {
     let third_write_ptr = cb.get_write_ptr().raw_addr(&cb);
     cb.add_comment("Ten bytes in");
 
-    assert_eq!(&vec!( "Beginning".to_string() ), cb.comments_at(first_write_ptr).unwrap());
-    assert_eq!(&vec!( "Two bytes in".to_string(), "Still two bytes in".to_string() ), cb.comments_at(second_write_ptr).unwrap());
-    assert_eq!(&vec!( "Ten bytes in".to_string() ), cb.comments_at(third_write_ptr).unwrap());
+    assert_eq!(&vec!( "Beginning".to_owned() ), cb.comments_at(first_write_ptr).unwrap());
+    assert_eq!(&vec!( "Two bytes in".to_owned(), "Still two bytes in".to_owned() ), cb.comments_at(second_write_ptr).unwrap());
+    assert_eq!(&vec!( "Ten bytes in".to_owned() ), cb.comments_at(third_write_ptr).unwrap());
 }

yjit/src/backend/arm64/mod.rs

@@ -1243,7 +1243,7 @@ impl Assembler
                     emit_cmp_zero_jump(cb, opnd.into(), false, compile_side_exit(*target, self, ocb)?);
                 },
                 Insn::IncrCounter { mem, value } => {
-                    let label = cb.new_label("incr_counter_loop".to_string());
+                    let label = cb.new_label("incr_counter_loop".into());
                     cb.write_label(label);
 
                     ldaxr(cb, Self::SCRATCH0, mem.into());

yjit/src/backend/ir.rs

@@ -186,7 +186,7 @@ impl Opnd
 
     /// Maps the indices from a previous list of instructions to a new list of
     /// instructions.
-    pub fn map_index(self, indices: &Vec<usize>) -> Opnd {
+    pub fn map_index(self, indices: &[usize]) -> Opnd {
         match self {
             Opnd::InsnOut { idx, num_bits } => {
                 Opnd::InsnOut { idx: indices[idx], num_bits }
@@ -1186,7 +1186,7 @@ impl Assembler
         assert!(!name.contains(' '), "use underscores in label names, not spaces");
 
         let label_idx = self.label_names.len();
-        self.label_names.push(name.to_string());
+        self.label_names.push(name.into());
         Target::Label(label_idx)
     }
 
@@ -1266,11 +1266,11 @@ impl Assembler
 
     /// Spill all live registers to the stack
     pub fn spill_regs(&mut self) {
-        self.spill_regs_except(&vec![]);
+        self.spill_regs_except(&[]);
     }
 
     /// Spill all live registers except `ignored_temps` to the stack
-    pub fn spill_regs_except(&mut self, ignored_temps: &Vec<RegOpnd>) {
+    pub fn spill_regs_except(&mut self, ignored_temps: &[RegOpnd]) {
         // Forget registers above the stack top
         let mut reg_mapping = self.ctx.get_reg_mapping();
         for stack_idx in self.ctx.get_stack_size()..MAX_CTX_TEMPS as u8 {
@@ -1348,17 +1348,17 @@ 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 reorder_reg_moves(old_moves: &Vec<(Reg, Opnd)>) -> Vec<(Reg, Opnd)> {
+    pub fn reorder_reg_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()
-            .filter(|&(reg, opnd)| Opnd::Reg(reg) != opnd).collect();
+        let mut old_moves: Vec<(Reg, Opnd)> = old_moves.into_iter()
+            .filter(|&&(reg, opnd)| Opnd::Reg(reg) != opnd).copied().collect();
 
         let mut new_moves = vec![];
         while old_moves.len() > 0 {
@@ -1385,6 +1385,7 @@ impl Assembler
     /// Sets the out field on the various instructions that require allocated
     /// registers because their output is used as the operand on a subsequent
     /// instruction. This is our implementation of the linear scan algorithm.
+    #[allow(clippy::useless_conversion)]
     pub(super) fn alloc_regs(mut self, regs: Vec<Reg>) -> Assembler
     {
         //dbg!(&self);
@@ -1394,7 +1395,7 @@ impl Assembler
 
         // Mutate the pool bitmap to indicate that the register at that index
         // has been allocated and is live.
-        fn alloc_reg(pool: &mut u32, regs: &Vec<Reg>) -> Option<Reg> {
+        fn alloc_reg(pool: &mut u32, regs: &[Reg]) -> Option<Reg> {
             for (index, reg) in regs.iter().enumerate() {
                 if (*pool & (1 << index)) == 0 {
                     *pool |= 1 << index;
@@ -1405,7 +1406,7 @@ impl Assembler
         }
 
         // Allocate a specific register
-        fn take_reg(pool: &mut u32, regs: &Vec<Reg>, reg: &Reg) -> Reg {
+        fn take_reg(pool: &mut u32, regs: &[Reg], reg: &Reg) -> Reg {
             let reg_index = regs.iter().position(|elem| elem.reg_no == reg.reg_no);
 
             if let Some(reg_index) = reg_index {
@@ -1419,7 +1420,7 @@ impl Assembler
         // Mutate the pool bitmap to indicate that the given register is being
         // returned as it is no longer used by the instruction that previously
         // held it.
-        fn dealloc_reg(pool: &mut u32, regs: &Vec<Reg>, reg: &Reg) {
+        fn dealloc_reg(pool: &mut u32, regs: &[Reg], reg: &Reg) {
             let reg_index = regs.iter().position(|elem| elem.reg_no == reg.reg_no);
 
             if let Some(reg_index) = reg_index {
@@ -1602,7 +1603,7 @@ impl Assembler
                 if c_args.len() > 0 {
                     // Resolve C argument dependencies
                     let c_args_len = c_args.len() as isize;
-                    let moves = Self::reorder_reg_moves(&c_args.drain(..).into_iter().collect());
+                    let moves = Self::reorder_reg_moves(&c_args.drain(..).into_iter().collect::<Vec<_>>());
                     shift_live_ranges(&mut shifted_live_ranges, asm.insns.len(), moves.len() as isize - c_args_len);
 
                     // Push batched C arguments
@@ -1751,7 +1752,7 @@ impl Assembler {
     }
 
     pub fn bake_string(&mut self, text: &str) {
-        self.push_insn(Insn::BakeString(text.to_string()));
+        self.push_insn(Insn::BakeString(text.into()));
     }
 
     #[allow(dead_code)]
@@ -1791,7 +1792,7 @@ impl Assembler {
     }
 
     /// Let vm_check_canary() assert the leafness of this ccall if leaf_ccall is set
-    fn set_stack_canary(&mut self, opnds: &Vec<Opnd>) -> Option<Opnd> {
+    fn set_stack_canary(&mut self, opnds: &[Opnd]) -> Option<Opnd> {
         // Use the slot right above the stack top for verifying leafness.
         let canary_opnd = self.stack_opnd(-1);
 

yjit/src/codegen.rs

@@ -43,6 +43,7 @@ type InsnGenFn = fn(
 
 /// Ephemeral code generation state.
 /// Represents a [crate::core::Block] while we build it.
+#[allow(clippy::type_complexity)]
 pub struct JITState<'a> {
     /// Instruction sequence for the compiling block
     pub iseq: IseqPtr,
@@ -403,7 +404,7 @@ impl<'a> JITState<'a> {
         if !self.perf_stack.is_empty() {
             self.perf_symbol_range_end(asm);
         }
-        self.perf_stack.push(symbol_name.to_string());
+        self.perf_stack.push(symbol_name.into());
         self.perf_symbol_range_start(asm, symbol_name);
     }
 
@@ -453,12 +454,9 @@ impl<'a> JITState<'a> {
 
     /// Return true if we're compiling a send-like instruction, not an opt_* instruction.
     pub fn is_sendish(&self) -> bool {
-        match unsafe { rb_iseq_opcode_at_pc(self.iseq, self.pc) } as u32 {
-            YARVINSN_send |
+        matches!(unsafe { rb_iseq_opcode_at_pc(self.iseq, self.pc) } as u32, YARVINSN_send |
             YARVINSN_opt_send_without_block |
-            YARVINSN_invokesuper => true,
-            _ => false,
-        }
+            YARVINSN_invokesuper)
     }
 
     /// Return the number of locals in the current ISEQ
@@ -1178,14 +1176,14 @@ pub fn gen_entry_reg_mapping(asm: &mut Assembler, blockid: BlockId, stack_size:
     // Find an existing callee block. If it's not found or uses no register, skip loading registers.
     let mut ctx = Context::default();
     ctx.set_stack_size(stack_size);
-    let reg_mapping = find_most_compatible_reg_mapping(blockid, &ctx).unwrap_or(RegMapping::default());
+    let reg_mapping = find_most_compatible_reg_mapping(blockid, &ctx).unwrap_or_default();
     if reg_mapping == RegMapping::default() {
         return reg_mapping;
     }
 
     // If found, load the same registers to reuse the block.
     asm_comment!(asm, "reuse maps: {:?}", reg_mapping);
-    let local_table_size: u32 = unsafe { get_iseq_body_local_table_size(blockid.iseq) }.try_into().unwrap();
+    let local_table_size: u32 = unsafe { get_iseq_body_local_table_size(blockid.iseq) };
     for &reg_opnd in reg_mapping.get_reg_opnds().iter() {
         match reg_opnd {
             RegOpnd::Local(local_idx) => {
@@ -1299,7 +1297,7 @@ pub fn gen_single_block(
     #[cfg(feature = "disasm")]
     if get_option_ref!(dump_disasm).is_some() {
         let blockid_idx = blockid.idx;
-        let chain_depth = if asm.ctx.get_chain_depth() > 0 { format!("(chain_depth: {})", asm.ctx.get_chain_depth()) } else { "".to_string() };
+        let chain_depth = if asm.ctx.get_chain_depth() > 0 { format!("(chain_depth: {})", asm.ctx.get_chain_depth()) } else { String::new() };
         asm_comment!(asm, "Block: {} {}", iseq_get_location(blockid.iseq, blockid_idx), chain_depth);
         asm_comment!(asm, "reg_mapping: {:?}", asm.ctx.get_reg_mapping());
     }
@@ -7708,7 +7706,7 @@ fn gen_send_iseq(
         // runtime guards later in copy_splat_args_for_rest_callee()
         if !iseq_has_rest {
             let supplying = argc - 1 - i32::from(kw_splat) + array_length as i32;
-            if (required_num..=required_num + opt_num).contains(&supplying) == false {
+            if !(required_num..=required_num + opt_num).contains(&supplying) {
                 gen_counter_incr(jit, asm, Counter::send_iseq_splat_arity_error);
                 return None;
             }
@@ -9546,7 +9544,7 @@ fn get_class_name(class: Option<VALUE>) -> String {
         unsafe { RB_TYPE_P(class, RUBY_T_MODULE) || RB_TYPE_P(class, RUBY_T_CLASS) }
     }).and_then(|class| unsafe {
         cstr_to_rust_string(rb_class2name(class))
-    }).unwrap_or_else(|| "Unknown".to_string())
+    }).unwrap_or_else(|| "Unknown".into())
 }
 
 /// Assemble "{class_name}#{method_name}" from a class pointer and a method ID
@@ -9556,15 +9554,15 @@ fn get_method_name(class: Option<VALUE>, mid: u64) -> String {
         unsafe { cstr_to_rust_string(rb_id2name(mid)) }
     } else {
         None
-    }.unwrap_or_else(|| "Unknown".to_string());
+    }.unwrap_or_else(|| "Unknown".into());
     format!("{}#{}", class_name, method_name)
 }
 
 /// Assemble "{label}@{iseq_path}:{lineno}" (iseq_inspect() format) from an ISEQ
 fn get_iseq_name(iseq: IseqPtr) -> String {
     let c_string = unsafe { rb_yjit_iseq_inspect(iseq) };
     let string = unsafe { CStr::from_ptr(c_string) }.to_str()
-        .unwrap_or_else(|_| "not UTF-8").to_string();
+        .unwrap_or_else(|_| "not UTF-8").into();
     unsafe { ruby_xfree(c_string as *mut c_void); }
     string
 }