Cloning: move arguments to create() into temporary locals

In C++ argument evaluation order is undefined. MSVC and Clang evaluate these in different orders, leading to hilarity when writing tests that expect a deterministic ordering.

Pull out all the argument expressions to create() in the clone functions so a cloned program is deterministic in its ordering between compilers.

Change-Id: I8e2de31398960c480ce7ee1dfaac4f67652d2dbc
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41544
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Auto-Submit: Ben Clayton <bclayton@google.com>

src/transform/bound_array_accessors.cc

@@ -120,12 +120,14 @@ ast::ArrayAccessorExpression* BoundArrayAccessors::Transform(
       return nullptr;
     }
   } else {
-    new_idx =
-        b.Call("min", b.Construct<u32>(ctx->Clone(old_idx)), b.Expr(size - 1));
+    auto* cloned_idx = ctx->Clone(old_idx);
+    new_idx = b.Call("min", b.Construct<u32>(cloned_idx), b.Expr(size - 1));
   }
 
-  return b.create<ast::ArrayAccessorExpression>(
-      ctx->Clone(expr->source()), ctx->Clone(expr->array()), new_idx);
+  // Clone arguments outside of create() call to have deterministic ordering
+  auto src = ctx->Clone(expr->source());
+  auto* arr = ctx->Clone(expr->array());
+  return b.create<ast::ArrayAccessorExpression>(src, arr, new_idx);
 }
 
 }  // namespace transform

src/transform/first_index_offset.cc

@@ -67,14 +67,15 @@ constexpr char kIndexOffsetPrefix[] = "tint_first_index_offset_";
 ast::Variable* clone_variable_with_new_name(CloneContext* ctx,
                                             ast::Variable* in,
                                             std::string new_name) {
+  // Clone arguments outside of create() call to have deterministic ordering
+  auto source = ctx->Clone(in->source());
+  auto symbol = ctx->dst->Symbols().Register(new_name);
+  auto* type = ctx->Clone(in->type());
+  auto* constructor = ctx->Clone(in->constructor());
+  auto decorations = ctx->Clone(in->decorations());
   return ctx->dst->create<ast::Variable>(
-      ctx->Clone(in->source()),                // source
-      ctx->dst->Symbols().Register(new_name),  // symbol
-      in->declared_storage_class(),            // declared_storage_class
-      ctx->Clone(in->type()),                  // type
-      in->is_const(),                          // is_const
-      ctx->Clone(in->constructor()),           // constructor
-      ctx->Clone(in->decorations()));          // decorations
+      source, symbol, in->declared_storage_class(), type, in->is_const(),
+      constructor, decorations);
 }
 
 }  // namespace

src/transform/transform.cc

@@ -40,12 +40,16 @@ ast::Function* Transform::CloneWithStatementsAtStart(
   for (auto* s : *in->body()) {
     statements.emplace_back(ctx->Clone(s));
   }
-  return ctx->dst->create<ast::Function>(
-      ctx->Clone(in->source()), ctx->Clone(in->symbol()),
-      ctx->Clone(in->params()), ctx->Clone(in->return_type()),
-      ctx->dst->create<ast::BlockStatement>(ctx->Clone(in->body()->source()),
-                                            statements),
-      ctx->Clone(in->decorations()));
+  // Clone arguments outside of create() call to have deterministic ordering
+  auto source = ctx->Clone(in->source());
+  auto symbol = ctx->Clone(in->symbol());
+  auto params = ctx->Clone(in->params());
+  auto* return_type = ctx->Clone(in->return_type());
+  auto* body = ctx->dst->create<ast::BlockStatement>(
+      ctx->Clone(in->body()->source()), statements);
+  auto decos = ctx->Clone(in->decorations());
+  return ctx->dst->create<ast::Function>(source, symbol, params, return_type,
+                                         body, decos);
 }
 
 }  // namespace transform