From f3a82989c4df85e93c2554d56dfd5d16505939af Mon Sep 17 00:00:00 2001
From: Jim Blandy <jimb@red-bean.com>
Date: Fri, 21 Jun 2024 16:25:51 -0700
Subject: [PATCH] Narrow clones in `upgrade_type`.

---
 naga/src/front/atomic_upgrade.rs | 66 ++++++++++++++++++--------------
 1 file changed, 38 insertions(+), 28 deletions(-)

diff --git a/naga/src/front/atomic_upgrade.rs b/naga/src/front/atomic_upgrade.rs
index effcbb03a0d..a6afea41104 100644
--- a/naga/src/front/atomic_upgrade.rs
+++ b/naga/src/front/atomic_upgrade.rs
@@ -40,6 +40,8 @@ use crate::{GlobalVariable, Handle, Module, Type, TypeInner};
 pub enum Error {
     #[error("encountered an unsupported expression")]
     Unsupported,
+    #[error("upgrading structs of more than one member is not yet implemented")]
+    MultiMemberStruct,
     #[error("encountered unsupported global initializer in an atomic variable")]
     GlobalInitUnsupported,
 }
@@ -103,9 +105,7 @@ impl<'a> UpgradeState<'a> {
         let padding = self.inc_padding();
         padding.trace("upgrading type: ", ty);
 
-        let r#type = self.module.types[ty].clone();
-
-        let inner = match r#type.inner.clone() {
+        let inner = match self.module.types[ty].inner {
             TypeInner::Scalar(scalar) => {
                 log::trace!("{padding}hit the scalar leaf, replacing with an atomic");
                 TypeInner::Atomic(scalar)
@@ -119,44 +119,54 @@ impl<'a> UpgradeState<'a> {
                 size,
                 stride,
             },
-            TypeInner::Struct { mut members, span } => TypeInner::Struct {
-                members: {
+            TypeInner::Struct { ref members, span } => {
+                match &members[..] {
                     // In the future we should have to figure out which member needs
                     // upgrading, but for now we'll only cover the single-member case
-                    if members.len() == 1 {
-                        members[0].ty = self.upgrade_type(members[0].ty)?;
+                    &[crate::StructMember {
+                        ref name,
+                        ty,
+                        ref binding,
+                        offset,
+                    }] => {
+                        // Take our own clones of these values now, so that
+                        // `upgrade_type` can mutate the module.
+                        let name = name.clone();
+                        let binding = binding.clone();
+                        let upgraded_member_type = self.upgrade_type(ty)?;
+                        TypeInner::Struct {
+                            members: vec![crate::StructMember {
+                                name,
+                                ty: upgraded_member_type,
+                                binding,
+                                offset,
+                            }],
+                            span,
+                        }
                     }
-                    members
-                },
-                span,
-            },
+                    _ => return Err(Error::MultiMemberStruct),
+                }
+            }
             TypeInner::BindingArray { base, size } => TypeInner::BindingArray {
                 base: self.upgrade_type(base)?,
                 size,
             },
-            n => n,
+            _ => return Ok(ty),
         };
 
+        // Now that we've upgraded any subtypes, re-borrow a reference to our
+        // type and update its `inner`.
+        let r#type = &self.module.types[ty];
+        let span = self.module.types.get_span(ty);
         let new_type = Type {
             name: r#type.name.clone(),
             inner,
         };
-        let new_ty = if let Some(prev_ty) = self.module.types.get(&new_type) {
-            padding.trace("type exists: ", prev_ty);
-            prev_ty
-        } else {
-            padding.debug("ty: ", ty);
-            padding.debug("from: ", &r#type);
-            padding.debug("to:   ", &new_type);
-
-            let new_ty = self
-                .module
-                .types
-                .insert(new_type, self.module.types.get_span(ty));
-            padding.debug("new ty: ", new_ty);
-            new_ty
-        };
-        Ok(new_ty)
+        padding.debug("ty: ", ty);
+        padding.debug("from: ", r#type);
+        padding.debug("to:   ", &new_type);
+        let new_handle = self.module.types.insert(new_type, span);
+        Ok(new_handle)
     }
 
     fn upgrade_global_variable(&mut self, handle: Handle<GlobalVariable>) -> Result<(), Error> {