From 9562dbbc9f0c4ecc7fe6dc39d2d93cad8faa1fe0 Mon Sep 17 00:00:00 2001 From: Laurence Tratt Date: Fri, 24 Jan 2025 14:50:01 +0000 Subject: [PATCH 1/3] Automated Clippy fixes in tests. --- ykrt/src/compile/jitc_yk/codegen/x64/mod.rs | 8 ++++---- ykrt/src/compile/jitc_yk/codegen/x64/rev_analyse.rs | 6 +++--- ykrt/src/compile/jitc_yk/jit_ir/parser.rs | 2 +- ykrt/src/compile/mod.rs | 6 +++--- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/ykrt/src/compile/jitc_yk/codegen/x64/mod.rs b/ykrt/src/compile/jitc_yk/codegen/x64/mod.rs index 456140ebb..6eb49899a 100644 --- a/ykrt/src/compile/jitc_yk/codegen/x64/mod.rs +++ b/ykrt/src/compile/jitc_yk/codegen/x64/mod.rs @@ -2803,7 +2803,7 @@ mod tests { let mut map = HashMap::new(); for (i, regs) in X64_RAW_REGS_MAP.iter().enumerate() { for r in regs.iter() { - if *r != "" { + if !r.is_empty() { map.insert(*r, i); } } @@ -2833,7 +2833,7 @@ mod tests { }; } - fn fmatcher<'a>(ptn: &'a str) -> FMatcher<'a> { + fn fmatcher(ptn: &str) -> FMatcher<'_> { let mut fmb = FMBuilder::new(ptn) .unwrap() .name_matching_validator(|names| { @@ -4714,11 +4714,11 @@ mod tests { m.push_param(loc.clone()); let pinst1: Inst = jit_ir::ParamInst::new(ParamIdx::try_from(0).unwrap(), m.int8_tyidx()).into(); - m.push(pinst1.clone()).unwrap(); + m.push(pinst1).unwrap(); m.push_param(loc); let pinst2: Inst = jit_ir::ParamInst::new(ParamIdx::try_from(1).unwrap(), m.int8_tyidx()).into(); - m.push(pinst2.clone()).unwrap(); + m.push(pinst2).unwrap(); let op1 = m.push_and_make_operand(pinst1).unwrap(); let op2 = m.push_and_make_operand(pinst2).unwrap(); diff --git a/ykrt/src/compile/jitc_yk/codegen/x64/rev_analyse.rs b/ykrt/src/compile/jitc_yk/codegen/x64/rev_analyse.rs index b0c1ca7c4..dd9788930 100644 --- a/ykrt/src/compile/jitc_yk/codegen/x64/rev_analyse.rs +++ b/ykrt/src/compile/jitc_yk/codegen/x64/rev_analyse.rs @@ -421,7 +421,7 @@ mod test { use std::assert_matches::assert_matches; use vob::vob; - fn rev_analyse_header<'a>(m: &'a Module) -> RevAnalyse<'a> { + fn rev_analyse_header(m: &Module) -> RevAnalyse<'_> { let mut rev_an = RevAnalyse::new(m); rev_an.analyse_header(); rev_an @@ -441,7 +441,7 @@ mod test { let rev_an = rev_analyse_header(&m); assert_eq!( rev_an.inst_vals_alive_until, - vec![3, 0, 0, 0] + [3, 0, 0, 0] .iter() .map(|x: &usize| InstIdx::try_from(*x).unwrap()) .collect::>() @@ -461,7 +461,7 @@ mod test { let rev_an = rev_analyse_header(&m); assert_eq!( rev_an.inst_vals_alive_until, - vec![2, 0, 5, 0, 0, 0] + [2, 0, 5, 0, 0, 0] .iter() .map(|x: &usize| InstIdx::try_from(*x).unwrap()) .collect::>() diff --git a/ykrt/src/compile/jitc_yk/jit_ir/parser.rs b/ykrt/src/compile/jitc_yk/jit_ir/parser.rs index a5eeca1f3..44cc39c66 100644 --- a/ykrt/src/compile/jitc_yk/jit_ir/parser.rs +++ b/ykrt/src/compile/jitc_yk/jit_ir/parser.rs @@ -149,7 +149,7 @@ impl<'lexer, 'input: 'lexer> JITIRParser<'lexer, 'input, '_> { Operand::Var(iidx) => Inst::Copy(iidx), Operand::Const(cidx) => Inst::Const(cidx), }; - self.push_assign(inst.into(), assign)?; + self.push_assign(inst, assign)?; } ASTInst::BinOp { assign, diff --git a/ykrt/src/compile/mod.rs b/ykrt/src/compile/mod.rs index 91dd4f25d..25e41a568 100644 --- a/ykrt/src/compile/mod.rs +++ b/ykrt/src/compile/mod.rs @@ -108,6 +108,9 @@ pub(crate) trait SideTraceInfo: fmt::Debug { fn as_any(self: Arc) -> Arc; } +#[cfg(test)] +pub(crate) use compiled_trace_testing::*; + #[cfg(test)] mod compiled_trace_testing { use super::*; @@ -201,6 +204,3 @@ mod compiled_trace_testing { } } } - -#[cfg(test)] -pub(crate) use compiled_trace_testing::*; From 054873d38aa18971ce974c96d96a085eaec02ecc Mon Sep 17 00:00:00 2001 From: Laurence Tratt Date: Fri, 24 Jan 2025 15:24:10 +0000 Subject: [PATCH 2/3] Manual fixes, partly suggested by Clippy. All of these caused Clippy warnings: Clippy didn't always make great suggestions for a fix. --- ykrt/src/compile/jitc_yk/aot_ir.rs | 4 +- ykrt/src/compile/jitc_yk/arbbitint.rs | 76 +++++++++++------------ ykrt/src/compile/jitc_yk/jit_ir/parser.rs | 8 +-- 3 files changed, 45 insertions(+), 43 deletions(-) diff --git a/ykrt/src/compile/jitc_yk/aot_ir.rs b/ykrt/src/compile/jitc_yk/aot_ir.rs index 1cd02e46f..e1ad50c99 100644 --- a/ykrt/src/compile/jitc_yk/aot_ir.rs +++ b/ykrt/src/compile/jitc_yk/aot_ir.rs @@ -2073,7 +2073,9 @@ mod tests { bytes, }; - let expect_bytes = rng.rev().map(|i| format!("{:02x}", i)).collect::(); + let expect_bytes = rng + .rev() + .fold("".to_string(), |acc, i| format!("{acc}{i:02x}")); let expect_usize = usize::from_str_radix(&expect_bytes, 16).unwrap(); assert_eq!( format!("{}", cp.display(&m)), diff --git a/ykrt/src/compile/jitc_yk/arbbitint.rs b/ykrt/src/compile/jitc_yk/arbbitint.rs index 61d5932bd..3948182bb 100644 --- a/ykrt/src/compile/jitc_yk/arbbitint.rs +++ b/ykrt/src/compile/jitc_yk/arbbitint.rs @@ -265,13 +265,13 @@ mod tests { assert_eq!( ArbBitInt::from_i64(8, x as i64) .wrapping_add(&ArbBitInt::from_i64(8, y as i64)).to_sign_ext_i8(), - i8::try_from(x.wrapping_add(y)).ok() + Some(x.wrapping_add(y)) ); // i16 assert_eq!( ArbBitInt::from_i64(8, x as i64) .wrapping_add(&ArbBitInt::from_i64(8, y as i64)).to_sign_ext_i16(), - i16::try_from(x.wrapping_add(y)).ok() + Some(i16::from(x.wrapping_add(y))) ); // wrapping_sub @@ -279,13 +279,13 @@ mod tests { assert_eq!( ArbBitInt::from_i64(8, x as i64) .wrapping_sub(&ArbBitInt::from_i64(8, y as i64)).to_sign_ext_i8(), - i8::try_from(x.wrapping_sub(y)).ok() + Some(x.wrapping_sub(y)) ); // i16 assert_eq!( ArbBitInt::from_i64(8, x as i64) .wrapping_sub(&ArbBitInt::from_i64(8, y as i64)).to_sign_ext_i16(), - i16::try_from(x.wrapping_sub(y)).ok() + Some(i16::from(x.wrapping_sub(y))) ); // wrapping_mul @@ -293,26 +293,26 @@ mod tests { assert_eq!( ArbBitInt::from_i64(8, x as i64) .wrapping_mul(&ArbBitInt::from_i64(8, y as i64)).to_sign_ext_i8(), - i8::try_from(x.wrapping_mul(y)).ok() + Some(x.wrapping_mul(y)) ); // i16 assert_eq!( ArbBitInt::from_i64(8, x as i64) .wrapping_mul(&ArbBitInt::from_i64(8, y as i64)).to_sign_ext_i16(), - i16::try_from(x.wrapping_mul(y)).ok() + Some(i16::from(x.wrapping_mul(y))) ); // bitadd assert_eq!( ArbBitInt::from_i64(8, x as i64) .bitand(&ArbBitInt::from_i64(8, y as i64)).to_sign_ext_i8(), - i8::try_from(x.bitand(y)).ok() + Some(x.bitand(y)) ); // i16 assert_eq!( ArbBitInt::from_i64(8, x as i64) .bitand(&ArbBitInt::from_i64(8, y as i64)).to_sign_ext_i16(), - i16::try_from(x.bitand(y)).ok() + Some(i16::from(x.bitand(y))) ); // bitor @@ -320,13 +320,13 @@ mod tests { assert_eq!( ArbBitInt::from_i64(8, x as i64) .bitor(&ArbBitInt::from_i64(8, y as i64)).to_sign_ext_i8(), - i8::try_from(x.bitor(y)).ok() + Some(x.bitor(y)) ); // i16 assert_eq!( ArbBitInt::from_i64(8, x as i64) .bitor(&ArbBitInt::from_i64(8, y as i64)).to_sign_ext_i16(), - i16::try_from(x.bitor(y)).ok() + Some(i16::from(x.bitor(y))) ); // bitxor @@ -334,13 +334,13 @@ mod tests { assert_eq!( ArbBitInt::from_i64(8, x as i64) .bitxor(&ArbBitInt::from_i64(8, y as i64)).to_sign_ext_i8(), - i8::try_from(x.bitxor(y)).ok() + Some(x.bitxor(y)) ); // i16 assert_eq!( ArbBitInt::from_i64(8, x as i64) .bitxor(&ArbBitInt::from_i64(8, y as i64)).to_sign_ext_i16(), - i16::try_from(x.bitxor(y)).ok() + Some(i16::from(x.bitxor(y))) ); } @@ -364,7 +364,7 @@ mod tests { assert_eq!( ArbBitInt::from_i64(16, x as i64) .wrapping_add(&ArbBitInt::from_i64(16, y as i64)).to_sign_ext_i16(), - i16::try_from(x.wrapping_add(y)).ok() + Some(x.wrapping_add(y)) ); // wrapping_sub @@ -378,7 +378,7 @@ mod tests { assert_eq!( ArbBitInt::from_i64(16, x as i64) .wrapping_sub(&ArbBitInt::from_i64(16, y as i64)).to_sign_ext_i16(), - i16::try_from(x.wrapping_sub(y)).ok() + Some(x.wrapping_sub(y)) ); // wrapping_mul @@ -392,7 +392,7 @@ mod tests { assert_eq!( ArbBitInt::from_i64(16, x as i64) .wrapping_mul(&ArbBitInt::from_i64(16, y as i64)).to_sign_ext_i16(), - i16::try_from(x.wrapping_mul(y)).ok() + Some(x.wrapping_mul(y)) ); // bitand @@ -406,7 +406,7 @@ mod tests { assert_eq!( ArbBitInt::from_i64(16, x as i64) .bitand(&ArbBitInt::from_i64(16, y as i64)).to_sign_ext_i16(), - i16::try_from(x.bitand(y)).ok() + Some(x.bitand(y)) ); // bitor @@ -420,7 +420,7 @@ mod tests { assert_eq!( ArbBitInt::from_i64(16, x as i64) .bitor(&ArbBitInt::from_i64(16, y as i64)).to_sign_ext_i16(), - i16::try_from(x.bitor(y)).ok() + Some(x.bitor(y)) ); // bitxor @@ -434,7 +434,7 @@ mod tests { assert_eq!( ArbBitInt::from_i64(16, x as i64) .bitxor(&ArbBitInt::from_i64(16, y as i64)).to_sign_ext_i16(), - i16::try_from(x.bitxor(y)).ok() + Some(x.bitxor(y)) ); } @@ -469,7 +469,7 @@ mod tests { assert_eq!( ArbBitInt::from_i64(32, x as i64) .wrapping_add(&ArbBitInt::from_i64(32, y as i64)).to_sign_ext_i32(), - i32::try_from(x.wrapping_add(y)).ok() + Some(x.wrapping_add(y)) ); // wrapping_sub @@ -489,7 +489,7 @@ mod tests { assert_eq!( ArbBitInt::from_i64(32, x as i64) .wrapping_sub(&ArbBitInt::from_i64(32, y as i64)).to_sign_ext_i32(), - i32::try_from(x.wrapping_sub(y)).ok() + Some(x.wrapping_sub(y)) ); // wrapping_mul @@ -509,7 +509,7 @@ mod tests { assert_eq!( ArbBitInt::from_i64(32, x as i64) .wrapping_mul(&ArbBitInt::from_i64(32, y as i64)).to_sign_ext_i32(), - i32::try_from(x.wrapping_mul(y)).ok() + Some(x.wrapping_mul(y)) ); // bitand @@ -529,7 +529,7 @@ mod tests { assert_eq!( ArbBitInt::from_i64(32, x as i64) .bitand(&ArbBitInt::from_i64(32, y as i64)).to_sign_ext_i32(), - i32::try_from(x.bitand(y)).ok() + Some(x.bitand(y)) ); // bitor @@ -549,7 +549,7 @@ mod tests { assert_eq!( ArbBitInt::from_i64(32, x as i64) .bitor(&ArbBitInt::from_i64(32, y as i64)).to_sign_ext_i32(), - i32::try_from(x.bitor(y)).ok() + Some(x.bitor(y)) ); // bitxor @@ -569,7 +569,7 @@ mod tests { assert_eq!( ArbBitInt::from_i64(32, x as i64) .bitxor(&ArbBitInt::from_i64(32, y as i64)).to_sign_ext_i32(), - i32::try_from(x.bitxor(y)).ok() + Some(x.bitxor(y)) ); } @@ -615,7 +615,7 @@ mod tests { assert_eq!( ArbBitInt::from_i64(64, x) .wrapping_add(&ArbBitInt::from_i64(64, y)).to_sign_ext_i64(), - i64::try_from(x.wrapping_add(y)).ok() + Some(x.wrapping_add(y)) ); // wrapping_sub @@ -641,7 +641,7 @@ mod tests { assert_eq!( ArbBitInt::from_i64(64, x) .wrapping_sub(&ArbBitInt::from_i64(64, y)).to_sign_ext_i64(), - i64::try_from(x.wrapping_sub(y)).ok() + Some(x.wrapping_sub(y)) ); // wrapping_mul @@ -667,7 +667,7 @@ mod tests { assert_eq!( ArbBitInt::from_i64(64, x) .wrapping_mul(&ArbBitInt::from_i64(64, y)).to_sign_ext_i64(), - i64::try_from(x.wrapping_mul(y)).ok() + Some(x.wrapping_mul(y)) ); // bitand @@ -693,7 +693,7 @@ mod tests { assert_eq!( ArbBitInt::from_i64(64, x) .bitand(&ArbBitInt::from_i64(64, y)).to_sign_ext_i64(), - i64::try_from(x.bitand(y)).ok() + Some(x.bitand(y)) ); // bitor @@ -719,7 +719,7 @@ mod tests { assert_eq!( ArbBitInt::from_i64(64, x) .bitor(&ArbBitInt::from_i64(64, y)).to_sign_ext_i64(), - i64::try_from(x.bitor(y)).ok() + Some(x.bitor(y)) ); // bitxor @@ -745,7 +745,7 @@ mod tests { assert_eq!( ArbBitInt::from_i64(64, x) .bitxor(&ArbBitInt::from_i64(64, y)).to_sign_ext_i64(), - i64::try_from(x.bitxor(y)).ok() + Some(x.bitxor(y)) ); } @@ -756,7 +756,7 @@ mod tests { // "failure cases only". assert_eq!( ArbBitInt::from_u64(8, x as u64).checked_shl(y).map(|x| x.to_zero_ext_u8()), - x.checked_shl(y).map(|x| u8::try_from(x).ok()) + x.checked_shl(y).map(Some) ); } @@ -767,7 +767,7 @@ mod tests { // "failure cases only". assert_eq!( ArbBitInt::from_u64(16, x as u64).checked_shl(y).map(|x| x.to_zero_ext_u16()), - x.checked_shl(y).map(|x| u16::try_from(x).ok()) + x.checked_shl(y).map(Some) ); } @@ -778,7 +778,7 @@ mod tests { // "failure cases only". assert_eq!( ArbBitInt::from_u64(32, x as u64).checked_shl(y).map(|x| x.to_zero_ext_u32()), - x.checked_shl(y).map(|x| u32::try_from(x).ok()) + x.checked_shl(y).map(Some) ); } @@ -789,7 +789,7 @@ mod tests { // "failure cases only". assert_eq!( ArbBitInt::from_u64(64, x).checked_shl(y).map(|x| x.to_zero_ext_u64()), - x.checked_shl(y).map(|x| u64::try_from(x).ok()) + x.checked_shl(y).map(Some) ); } @@ -800,7 +800,7 @@ mod tests { // "failure cases only". assert_eq!( ArbBitInt::from_u64(8, x as u64).checked_shr(y).map(|x| x.to_zero_ext_u8()), - x.checked_shr(y).map(|x| u8::try_from(x).ok()) + x.checked_shr(y).map(Some) ); } @@ -811,7 +811,7 @@ mod tests { // "failure cases only". assert_eq!( ArbBitInt::from_u64(16, x as u64).checked_shr(y).map(|x| x.to_zero_ext_u16()), - x.checked_shr(y).map(|x| u16::try_from(x).ok()) + x.checked_shr(y).map(Some) ); } @@ -822,7 +822,7 @@ mod tests { // "failure cases only". assert_eq!( ArbBitInt::from_u64(32, x as u64).checked_shr(y).map(|x| x.to_zero_ext_u32()), - x.checked_shr(y).map(|x| u32::try_from(x).ok()) + x.checked_shr(y).map(Some) ); } @@ -833,7 +833,7 @@ mod tests { // "failure cases only". assert_eq!( ArbBitInt::from_u64(64, x).checked_shr(y).map(|x| x.to_zero_ext_u64()), - x.checked_shr(y).map(|x| u64::try_from(x).ok()) + x.checked_shr(y).map(Some) ); } } diff --git a/ykrt/src/compile/jitc_yk/jit_ir/parser.rs b/ykrt/src/compile/jitc_yk/jit_ir/parser.rs index 44cc39c66..87edbc5cc 100644 --- a/ykrt/src/compile/jitc_yk/jit_ir/parser.rs +++ b/ykrt/src/compile/jitc_yk/jit_ir/parser.rs @@ -317,7 +317,7 @@ impl<'lexer, 'input: 'lexer> JITIRParser<'lexer, 'input, '_> { .span_str(tiidx) .parse::() .map_err(|e| self.error_at_span(tiidx, &e.to_string()))?; - assert_eq!(self.m.params.len(), usize::try_from(off).unwrap()); + assert_eq!(self.m.params.len(), off); let type_ = self.process_type(type_)?; let size = self.m.type_(type_).byte_size().ok_or_else(|| { self.error_at_span( @@ -506,7 +506,7 @@ impl<'lexer, 'input: 'lexer> JITIRParser<'lexer, 'input, '_> { } in func_types { let name = self.lexer.span_str(name_span).to_owned(); - if self.func_types_map.get(&name).is_some() { + if self.func_types_map.contains_key(&name) { return Err(format!("Duplicate function type '{name}'").into()); } let arg_tys = { @@ -535,7 +535,7 @@ impl<'lexer, 'input: 'lexer> JITIRParser<'lexer, 'input, '_> { } in func_decls { let name = self.lexer.span_str(name_span).to_owned(); - if self.func_types_map.get(&name).is_some() { + if self.func_types_map.contains_key(&name) { todo!(); } let arg_tys = { @@ -694,7 +694,7 @@ impl<'lexer, 'input: 'lexer> JITIRParser<'lexer, 'input, '_> { } self.m.push(inst).unwrap(); - debug_assert!(self.inst_idx_map.get(&iidx).is_none()); + debug_assert!(!self.inst_idx_map.contains_key(&iidx)); self.inst_idx_map.insert(iidx, self.m.last_inst_idx()); Ok(()) } From 0db08144d28178dffbfbdcf7eed215c17e05f34c Mon Sep 17 00:00:00 2001 From: Laurence Tratt Date: Fri, 24 Jan 2025 15:29:41 +0000 Subject: [PATCH 3/3] Run Clippy on the test suite too. --- .buildbot.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.buildbot.sh b/.buildbot.sh index 424bd9260..b740996d0 100644 --- a/.buildbot.sh +++ b/.buildbot.sh @@ -137,7 +137,7 @@ for tracer in ${TRACERS}; do echo "$WARNING_DEFINES" | xargs cargo rustc -p xtask --profile check --bin xtask -- # Error if Clippy detects any warnings introduced in lines changed in this PR. - cargo-clippy-diff origin/master -- --all-features -- -D warnings + cargo-clippy-diff origin/master -- --all-features --tests -- -D warnings done # Run the tests multiple times on hwt to try and catch non-deterministic