Use workspace to set clippy linters

use them in yjit and zjit
fix clippy issues

Author: jesse-shopify

Cargo.toml

@@ -49,3 +49,19 @@ 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 }
+
+too_many_arguments = "allow"
+identity_op = "allow"
+
+unwrap_or_default = "forbid"
+ptr_arg = "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/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
@@ -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/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,
@@ -1178,14 +1179,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) => {
@@ -7708,7 +7709,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;
             }

yjit/src/core.rs

@@ -1099,7 +1099,7 @@ impl Context {
                 MapToLocal(local_idx) => {
                     bits.push_op(CtxOp::MapTempLocal);
                     bits.push_u3(stack_idx as u8);
-                    bits.push_u3(local_idx as u8);
+                    bits.push_u3(local_idx);
                 }
 
                 MapToSelf => {
@@ -1170,19 +1170,19 @@ impl Context {
 
             match op {
                 CtxOp::SetSelfType => {
-                    ctx.self_type = unsafe { transmute(bits.read_u4(&mut idx)) };
+                    ctx.self_type = unsafe { transmute::<u8, Type>(bits.read_u4(&mut idx)) };
                 }
 
                 CtxOp::SetLocalType => {
                     let local_idx = bits.read_u3(&mut idx) as usize;
-                    let t = unsafe { transmute(bits.read_u4(&mut idx)) };
+                    let t = unsafe { transmute::<u8, Type>(bits.read_u4(&mut idx)) };
                     ctx.set_local_type(local_idx, t);
                 }
 
                 // Map temp to stack (known type)
                 CtxOp::SetTempType => {
                     let temp_idx = bits.read_u3(&mut idx) as usize;
-                    let temp_type = unsafe { transmute(bits.read_u4(&mut idx)) };
+                    let temp_type = unsafe { transmute::<u8, Type>(bits.read_u4(&mut idx)) };
                     ctx.set_temp_mapping(temp_idx, TempMapping::MapToStack(temp_type));
                 }
 
@@ -2294,7 +2294,7 @@ pub fn limit_block_versions(blockid: BlockId, ctx: &Context) -> Context {
         let generic_ctx = ctx.get_generic_ctx();
 
         if cfg!(debug_assertions) {
-            let mut ctx = ctx.clone();
+            let mut ctx = *ctx;
             if ctx.inline() {
                 // Suppress TypeDiff::Incompatible from ctx.diff(). We return TypeDiff::Incompatible
                 // to keep inlining blocks until we hit the limit, but it's safe to give up inlining.
@@ -2920,7 +2920,7 @@ impl Context {
         }
 
         // Prepare a Context with the same registers
-        let mut dst_with_same_regs = dst.clone();
+        let mut dst_with_same_regs = *dst;
         dst_with_same_regs.set_reg_mapping(self.get_reg_mapping());
 
         // Diff registers and other stuff separately, and merge them
@@ -3823,7 +3823,7 @@ pub fn gen_branch_stub_hit_trampoline(ocb: &mut OutlinedCb) -> Option<CodePtr> {
 }
 
 /// Return registers to be pushed and popped on branch_stub_hit.
-pub fn caller_saved_temp_regs() -> impl Iterator<Item = &'static Reg> + DoubleEndedIterator {
+pub fn caller_saved_temp_regs() -> impl DoubleEndedIterator<Item = &'static Reg> {
     let temp_regs = Assembler::get_temp_regs().iter();
     let len = temp_regs.len();
     // The return value gen_leave() leaves in C_RET_REG

yjit/src/cruby.rs

@@ -97,7 +97,7 @@ pub type size_t = u64;
 pub type RedefinitionFlag = u32;
 
 #[allow(dead_code)]
-#[allow(clippy::all)]
+#[allow(clippy::complexity)]
 mod autogened {
     use super::*;
     // Textually include output from rust-bindgen as suggested by its user guide.

yjit/src/lib.rs

@@ -1,8 +1,3 @@
-// Clippy disagreements
-#![allow(clippy::style)] // We are laid back about style
-#![allow(clippy::too_many_arguments)] // :shrug:
-#![allow(clippy::identity_op)] // Sometimes we do it for style
-
 // TODO(alan): This lint is right -- the way we use `static mut` is UB happy. We have many globals
 // and take `&mut` frequently, sometimes with a method that easily allows calling it twice.
 //

yjit/src/utils.rs

@@ -92,10 +92,7 @@ pub fn ruby_str_to_rust(v: VALUE) -> String {
     let str_ptr = unsafe { rb_RSTRING_PTR(v) } as *mut u8;
     let str_len: usize = unsafe { rb_RSTRING_LEN(v) }.try_into().unwrap();
     let str_slice: &[u8] = unsafe { slice::from_raw_parts(str_ptr, str_len) };
-    match String::from_utf8(str_slice.to_vec()) {
-        Ok(utf8) => utf8,
-        Err(_) => String::new(),
-    }
+    String::from_utf8(str_slice.to_vec()).unwrap_or_default()
 }
 
 // Location is the file defining the method, colon, method name.

zjit/Cargo.toml

@@ -19,3 +19,6 @@ expect-test = "1.5.1"
 # Support --yjit-dump-disasm and RubyVM::YJIT.disasm using libcapstone.
 disasm = ["capstone"]
 runtime_checks = []
+
+[lints]
+workspace = true

zjit/src/backend/arm64/mod.rs

@@ -139,7 +139,7 @@ fn emit_load_value(cb: &mut CodeBlock, rd: A64Opnd, value: u64) -> usize {
         // If the value fits into a single movz
         // instruction, then we'll use that.
         movz(cb, rd, A64Opnd::new_uimm(current), 0);
-        return 1;
+        1
     } else if u16::try_from(!value).is_ok() {
         // For small negative values, use a single movn
         movn(cb, rd, A64Opnd::new_uimm(!value), 0);
@@ -182,7 +182,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,
@@ -480,7 +480,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.
@@ -1106,8 +1106,8 @@ impl Assembler
                     // be stored is first and the address is second. However in
                     // our IR we have the address first and the register second.
                     match dest_num_bits {
-                        64 | 32 => stur(cb, src.into(), dest.into()),
-                        16 => sturh(cb, src.into(), dest.into()),
+                        64 | 32 => stur(cb, src, dest),
+                        16 => sturh(cb, src, dest),
                         num_bits => panic!("unexpected dest num_bits: {} (src: {:#?}, dest: {:#?})", num_bits, src, dest),
                     }
                 },
@@ -1411,7 +1411,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,