Migrate to Rust 2024, and fix clippy warnings (from newer nightlies). (#21)

A bit late on this because my default `nightly` ended up falling behind
Rust-GPU's own toolchain, but I'm guessing landing any other PR will
need this first anyway (due to CI).

Author: eddyb

Cargo.toml

@@ -5,7 +5,7 @@ repository = "https://github.com/rust-gpu/spirt"
 homepage = "https://github.com/rust-gpu/spirt"
 version = "0.4.0"
 authors = ["SPIR-T developers", "Embark <[email protected]>"]
-edition = "2021"
+edition = "2024"
 license = "MIT OR Apache-2.0"
 readme = "README.md"
 # FIXME(eddyb) should this point to the version built from `git`?

src/cfg.rs

@@ -82,7 +82,7 @@ impl ControlFlowGraph {
     pub fn rev_post_order(
         &self,
         func_def_body: &FuncDefBody,
-    ) -> impl DoubleEndedIterator<Item = Region> {
+    ) -> impl DoubleEndedIterator<Item = Region> + use<> {
         let mut post_order = SmallVec::<[_; 8]>::new();
         self.traverse_whole_func(
             func_def_body,
@@ -1871,14 +1871,14 @@ impl<'a> Structurizer<'a> {
         // but instead make all `Region`s entirely hermetic wrt inputs.
         #[allow(clippy::manual_flatten)]
         for case in cases {
-            if let Ok(ClaimedRegion { structured_body, structured_body_inputs, .. }) = case {
-                if !structured_body_inputs.is_empty() {
-                    self.region_input_rewrites.insert(
-                        structured_body,
-                        RegionInputRewrites::ReplaceWith(structured_body_inputs),
-                    );
-                    self.func_def_body.at_mut(structured_body).def().inputs.clear();
-                }
+            if let Ok(ClaimedRegion { structured_body, structured_body_inputs, .. }) = case
+                && !structured_body_inputs.is_empty()
+            {
+                self.region_input_rewrites.insert(
+                    structured_body,
+                    RegionInputRewrites::ReplaceWith(structured_body_inputs),
+                );
+                self.func_def_body.at_mut(structured_body).def().inputs.clear();
             }
         }
 

src/cfgssa.rs

@@ -564,7 +564,7 @@ mod data {
             self.contains(k).then(|| self.entry(k).remove().unwrap())
         }
 
-        pub fn keys(&self) -> impl Iterator<Item = K> {
+        pub fn keys(&self) -> impl Iterator<Item = K> + use<K, V, VS> {
             let mut i = 0;
             let mut remaining = self.occupied;
             iter::from_fn(move || {

src/passes/link.rs

@@ -95,10 +95,10 @@ impl Visitor<'_> for LiveExportCollector<'_> {
         match *import {
             Import::LinkName(name) => {
                 let export_key = ExportKey::LinkName(name);
-                if let Some(&exportee) = self.module.exports.get(&export_key) {
-                    if self.live_exports.insert(export_key) {
-                        exportee.inner_visit_with(self);
-                    }
+                if let Some(&exportee) = self.module.exports.get(&export_key)
+                    && self.live_exports.insert(export_key)
+                {
+                    exportee.inner_visit_with(self);
                 }
             }
         }
@@ -198,12 +198,11 @@ impl Visitor<'_> for ImportResolutionCollector<'_> {
             // FIXME(eddyb) if the export is missing (or the wrong kind), it will
             // simply not get remapped - perhaps some kind of diagnostic is in
             // order? (maybe an entire pass infrastructure that can report errors)
-            if let DeclDef::Imported(Import::LinkName(name)) = gv_decl.def {
-                if let Some(&Exportee::GlobalVar(def_gv)) =
+            if let DeclDef::Imported(Import::LinkName(name)) = gv_decl.def
+                && let Some(&Exportee::GlobalVar(def_gv)) =
                     self.module.exports.get(&ExportKey::LinkName(name))
-                {
-                    self.resolved_global_vars.insert(gv, def_gv);
-                }
+            {
+                self.resolved_global_vars.insert(gv, def_gv);
             }
         }
     }
@@ -215,12 +214,11 @@ impl Visitor<'_> for ImportResolutionCollector<'_> {
             // FIXME(eddyb) if the export is missing (or the wrong kind), it will
             // simply not get remapped - perhaps some kind of diagnostic is in
             // order? (maybe an entire pass infrastructure that can report errors)
-            if let DeclDef::Imported(Import::LinkName(name)) = func_decl.def {
-                if let Some(&Exportee::Func(def_func)) =
+            if let DeclDef::Imported(Import::LinkName(name)) = func_decl.def
+                && let Some(&Exportee::Func(def_func)) =
                     self.module.exports.get(&ExportKey::LinkName(name))
-                {
-                    self.resolved_funcs.insert(func, def_func);
-                }
+            {
+                self.resolved_funcs.insert(func, def_func);
             }
         }
     }

src/print/mod.rs

@@ -630,13 +630,13 @@ impl<'a> Visitor<'a> for Plan<'a> {
     }
 
     fn visit_func_decl(&mut self, func_decl: &'a FuncDecl) {
-        if let DeclDef::Present(func_def_body) = &func_decl.def {
-            if let Some(cfg) = &func_def_body.unstructured_cfg {
-                for region in cfg.rev_post_order(func_def_body) {
-                    if let Some(control_inst) = cfg.control_inst_on_exit_from.get(region) {
-                        for &target in &control_inst.targets {
-                            *self.use_counts.entry(Use::RegionLabel(target)).or_default() += 1;
-                        }
+        if let DeclDef::Present(func_def_body) = &func_decl.def
+            && let Some(cfg) = &func_def_body.unstructured_cfg
+        {
+            for region in cfg.rev_post_order(func_def_body) {
+                if let Some(control_inst) = cfg.control_inst_on_exit_from.get(region) {
+                    for &target in &control_inst.targets {
+                        *self.use_counts.entry(Use::RegionLabel(target)).or_default() += 1;
                     }
                 }
             }
@@ -1952,7 +1952,7 @@ impl AttrsAndDef {
         let mut maybe_def_end_anchor = pretty::Fragment::default();
         let mut name = name.into();
         if let [
-            pretty::Node::Anchor { is_def: ref mut original_anchor_is_def @ true, anchor, text: _ },
+            pretty::Node::Anchor { is_def: original_anchor_is_def @ true, anchor, text: _ },
             ..,
         ] = &mut name.nodes[..]
         {
@@ -4089,12 +4089,12 @@ impl Print for FuncAt<'_, DataInst> {
                             }
                             ConstKind::SpvInst { spv_inst_and_const_inputs } => {
                                 let (spv_inst, _const_inputs) = &**spv_inst_and_const_inputs;
-                                if spv_inst.opcode == wk.OpConstant {
-                                    if let [spv::Imm::Short(_, x)] = spv_inst.imms[..] {
-                                        // HACK(eddyb) only allow unambiguously positive values.
-                                        if i32::try_from(x).and_then(u32::try_from) == Ok(x) {
-                                            return Some(PseudoImm::U32(x));
-                                        }
+                                if spv_inst.opcode == wk.OpConstant
+                                    && let [spv::Imm::Short(_, x)] = spv_inst.imms[..]
+                                {
+                                    // HACK(eddyb) only allow unambiguously positive values.
+                                    if i32::try_from(x).and_then(u32::try_from) == Ok(x) {
+                                        return Some(PseudoImm::U32(x));
                                     }
                                 }
                             }

src/qptr/analyze.rs

@@ -533,10 +533,10 @@ impl MemTypeLayout {
         // "Fast accept" based on type alone (expected as recursion base case).
         if let QPtrMemUsageKind::StrictlyTyped(usage_type)
         | QPtrMemUsageKind::DirectAccess(usage_type) = usage.kind
+            && usage_offset == 0
+            && self.original_type == usage_type
         {
-            if usage_offset == 0 && self.original_type == usage_type {
-                return true;
-            }
+            return true;
         }
 
         {
@@ -608,7 +608,7 @@ impl MemTypeLayout {
                 let usage_fixed_len = usage
                     .max_size
                     .map(|size| {
-                        if size % usage_stride.get() != 0 {
+                        if !size.is_multiple_of(usage_stride.get()) {
                             // FIXME(eddyb) maybe this should be propagated up,
                             // as a sign that `usage` is malformed?
                             return Err(());
@@ -924,11 +924,11 @@ impl<'a> InferUsage<'a> {
                                 ));
                             }
                         };
-                        if data_inst_def.output_type.is_some_and(is_qptr) {
-                            if let Some(usage) = output_usage {
-                                usage_or_err_attrs_to_attach
-                                    .push((Value::DataInstOutput(data_inst), usage));
-                            }
+                        if data_inst_def.output_type.is_some_and(is_qptr)
+                            && let Some(usage) = output_usage
+                        {
+                            usage_or_err_attrs_to_attach
+                                .push((Value::DataInstOutput(data_inst), usage));
                         }
                     }
 

src/qptr/lift.rs

@@ -245,7 +245,7 @@ impl<'a> LiftToSpvPtrs<'a> {
                 let fixed_len = usage
                     .max_size
                     .map(|size| {
-                        if size % stride.get() != 0 {
+                        if !size.is_multiple_of(stride.get()) {
                             return Err(LiftError(Diag::bug([format!(
                                 "DynOffsetBase: size ({size}) not a multiple of stride ({stride})"
                             )
@@ -430,13 +430,13 @@ impl LiftToSpvPtrInstsInFunc<'_> {
         // FIXME(eddyb) maybe all this data should be packaged up together in a
         // type with fields like those of `DeferredPtrNoop` (or even more).
         let type_of_val_as_spv_ptr_with_layout = |v: Value| {
-            if let Value::DataInstOutput(v_data_inst) = v {
-                if let Some(ptr_noop) = self.deferred_ptr_noops.get(&v_data_inst) {
-                    return Ok((
-                        ptr_noop.output_pointer_addr_space,
-                        ptr_noop.output_pointee_layout.clone(),
-                    ));
-                }
+            if let Value::DataInstOutput(v_data_inst) = v
+                && let Some(ptr_noop) = self.deferred_ptr_noops.get(&v_data_inst)
+            {
+                return Ok((
+                    ptr_noop.output_pointer_addr_space,
+                    ptr_noop.output_pointee_layout.clone(),
+                ));
             }
 
             let (addr_space, pointee_type) =
@@ -685,17 +685,15 @@ impl LiftToSpvPtrInstsInFunc<'_> {
                 loop {
                     if let Components::Elements { stride: layout_stride, elem, fixed_len } =
                         &layout.components
+                        && layout_stride == stride
+                        && Ok(index_bounds.clone())
+                            == fixed_len
+                                .map(|len| i32::try_from(len.get()).map(|len| 0..len))
+                                .transpose()
                     {
-                        if layout_stride == stride
-                            && Ok(index_bounds.clone())
-                                == fixed_len
-                                    .map(|len| i32::try_from(len.get()).map(|len| 0..len))
-                                    .transpose()
-                        {
-                            access_chain_inputs.push(data_inst_def.inputs[1]);
-                            layout = elem.clone();
-                            break;
-                        }
+                        access_chain_inputs.push(data_inst_def.inputs[1]);
+                        layout = elem.clone();
+                        break;
                     }
 
                     // FIXME(eddyb) deduplicate with `maybe_adjust_pointer_for_access`.
@@ -1094,12 +1092,12 @@ impl Transformer for LiftToSpvPtrInstsInFunc<'_> {
                     if let DataInstKind::QPtr(_) = data_inst_def.kind {
                         lifted =
                             Err(LiftError(Diag::bug(["unimplemented qptr instruction".into()])));
-                    } else if let Some(ty) = data_inst_def.output_type {
-                        if matches!(self.lifter.cx[ty].kind, TypeKind::QPtr) {
-                            lifted = Err(LiftError(Diag::bug([
-                                "unimplemented qptr-producing instruction".into(),
-                            ])));
-                        }
+                    } else if let Some(ty) = data_inst_def.output_type
+                        && matches!(self.lifter.cx[ty].kind, TypeKind::QPtr)
+                    {
+                        lifted = Err(LiftError(Diag::bug([
+                            "unimplemented qptr-producing instruction".into(),
+                        ])));
                     }
                 }
                 match lifted {

src/qptr/lower.rs

@@ -625,16 +625,16 @@ impl LowerFromSpvPtrInstsInFunc<'_> {
                 );
             }
         }
-        if let Some(output_type) = data_inst_def.output_type {
-            if let Some((addr_space, pointee)) = self.lowerer.as_spv_ptr_type(output_type) {
-                old_and_new_attrs.get_or_insert_with(get_old_attrs).attrs.insert(
-                    QPtrAttr::FromSpvPtrOutput {
-                        addr_space: OrdAssertEq(addr_space),
-                        pointee: OrdAssertEq(pointee),
-                    }
-                    .into(),
-                );
-            }
+        if let Some(output_type) = data_inst_def.output_type
+            && let Some((addr_space, pointee)) = self.lowerer.as_spv_ptr_type(output_type)
+        {
+            old_and_new_attrs.get_or_insert_with(get_old_attrs).attrs.insert(
+                QPtrAttr::FromSpvPtrOutput {
+                    addr_space: OrdAssertEq(addr_space),
+                    pointee: OrdAssertEq(pointee),
+                }
+                .into(),
+            );
         }
 
         if let Some(LowerError(e)) = extra_error {

src/spv/lower.rs

@@ -799,12 +799,12 @@ impl Module {
 
                 Seq::Function
             };
-            if let Some(prev_seq) = seq {
-                if prev_seq > next_seq {
-                    return Err(invalid(&format!(
-                        "out of order: {next_seq:?} instructions must precede {prev_seq:?} instructions"
-                    )));
-                }
+            if let Some(prev_seq) = seq
+                && prev_seq > next_seq
+            {
+                return Err(invalid(&format!(
+                    "out of order: {next_seq:?} instructions must precede {prev_seq:?} instructions"
+                )));
             }
             seq = Some(next_seq);
 
@@ -978,26 +978,26 @@ impl Module {
                         local_id_defs.insert(id, local_id_def);
                     }
 
-                    if let Some(def_map) = &mut cfgssa_def_map {
-                        if let DeclDef::Present(func_def_body) = &func_decl.def {
-                            let current_block = match block_details.last() {
-                                Some((&current_block, _)) => current_block,
-                                // HACK(eddyb) ensure e.g. `OpFunctionParameter`
-                                // are treated like `OpPhi`s of the entry block.
-                                None => func_def_body.body,
-                            };
+                    if let Some(def_map) = &mut cfgssa_def_map
+                        && let DeclDef::Present(func_def_body) = &func_decl.def
+                    {
+                        let current_block = match block_details.last() {
+                            Some((&current_block, _)) => current_block,
+                            // HACK(eddyb) ensure e.g. `OpFunctionParameter`
+                            // are treated like `OpPhi`s of the entry block.
+                            None => func_def_body.body,
+                        };
 
-                            if opcode == wk.OpLabel {
-                                // HACK(eddyb) the entry block was already added.
-                                if current_block != func_def_body.body {
-                                    def_map.add_block(current_block);
-                                }
-                                continue;
+                        if opcode == wk.OpLabel {
+                            // HACK(eddyb) the entry block was already added.
+                            if current_block != func_def_body.body {
+                                def_map.add_block(current_block);
                             }
+                            continue;
+                        }
 
-                            if let Some(id) = result_id {
-                                def_map.add_def(current_block, id, result_type.unwrap());
-                            }
+                        if let Some(id) = result_id {
+                            def_map.add_def(current_block, id, result_type.unwrap());
                         }
                     }
                 }
@@ -1643,14 +1643,12 @@ impl Module {
             }
 
             // Sanity-check the entry block.
-            if let Some(func_def_body) = func_def_body {
-                if block_details[&func_def_body.body].phi_count > 0 {
-                    // FIXME(remove) embed IDs in errors by moving them to the
-                    // `let invalid = |...| ...;` closure that wraps insts.
-                    return Err(invalid(&format!(
-                        "in %{func_id}, the entry block contains `OpPhi`s"
-                    )));
-                }
+            if let Some(func_def_body) = func_def_body
+                && block_details[&func_def_body.body].phi_count > 0
+            {
+                // FIXME(remove) embed IDs in errors by moving them to the
+                // `let invalid = |...| ...;` closure that wraps insts.
+                return Err(invalid(&format!("in %{func_id}, the entry block contains `OpPhi`s")));
             }
         }
 

src/spv/read.rs

@@ -213,11 +213,11 @@ impl InstParser<'_> {
             self.inst.result_id = def.has_result_id.then(&mut id).transpose()?;
         }
 
-        if let Some(type_id) = self.inst.result_type_id {
-            if !self.known_ids.contains_key(&type_id) {
-                // FIXME(eddyb) also check that the ID is a valid type.
-                return Err(Error::UnknownResultTypeId(type_id));
-            }
+        if let Some(type_id) = self.inst.result_type_id
+            && !self.known_ids.contains_key(&type_id)
+        {
+            // FIXME(eddyb) also check that the ID is a valid type.
+            return Err(Error::UnknownResultTypeId(type_id));
         }
 
         for (mode, kind) in def.all_operands() {
@@ -266,7 +266,7 @@ impl ModuleParser {
     pub fn read_from_spv_bytes(spv_bytes: Vec<u8>) -> io::Result<Self> {
         let spv_spec = spec::Spec::get();
 
-        if spv_bytes.len() % 4 != 0 {
+        if !spv_bytes.len().is_multiple_of(4) {
             return Err(invalid("not a multiple of 4 bytes"));
         }
         // May need to mutate the bytes (to normalize endianness) later below.