writer/msl: Fix all tests that had unreachable AST nodes

By making nodes reachable, the resolver has now caught a whole lot of additional problems, which have been fixed in this CL.

Some of these broken tests were attempting to use private and workgroup variables as function-scope declarations.
This is not legal, and these have been moved to module-scope variables.

Bug: tint:469
Change-Id: I1fc3a10fa0e39e1c290a13323277d6e9257778c4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48048
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>

src/program_builder.h

@@ -594,6 +594,10 @@ class ProgramBuilder {
     return expr;
   }
 
+  /// Passthrough for nullptr
+  /// @return nullptr
+  ast::IdentifierExpression* Expr(std::nullptr_t) { return nullptr; }
+
   /// @param name the identifier name
   /// @return an ast::IdentifierExpression with the given name
   ast::IdentifierExpression* Expr(const std::string& name) {
@@ -606,6 +610,12 @@ class ProgramBuilder {
     return create<ast::IdentifierExpression>(symbol);
   }
 
+  /// @param variable the AST variable
+  /// @return an ast::IdentifierExpression with the variable's symbol
+  ast::IdentifierExpression* Expr(ast::Variable* variable) {
+    return create<ast::IdentifierExpression>(variable->symbol());
+  }
+
   /// @param source the source information
   /// @param name the identifier name
   /// @return an ast::IdentifierExpression with the given name
@@ -1252,9 +1262,10 @@ class ProgramBuilder {
   /// @param condition the else condition expression
   /// @param body the else body
   /// @returns the else statement pointer
-  ast::ElseStatement* Else(ast::Expression* condition,
+  template <typename CONDITION>
-                           ast::BlockStatement* body) {
+  ast::ElseStatement* Else(CONDITION&& condition, ast::BlockStatement* body) {
-    return create<ast::ElseStatement>(condition, body);
+    return create<ast::ElseStatement>(Expr(std::forward<CONDITION>(condition)),
+                                      body);
   }
 
   /// Creates a ast::IfStatement with input condition, body, and optional
@@ -1263,14 +1274,14 @@ class ProgramBuilder {
   /// @param body the if statement body
   /// @param elseStatements optional variadic else statements
   /// @returns the if statement pointer
-  template <typename... ElseStatements>
+  template <typename CONDITION, typename... ELSE_STATEMENTS>
-  ast::IfStatement* If(ast::Expression* condition,
+  ast::IfStatement* If(CONDITION&& condition,
                        ast::BlockStatement* body,
-                       ElseStatements&&... elseStatements) {
+                       ELSE_STATEMENTS&&... elseStatements) {
     return create<ast::IfStatement>(
-        condition, body,
+        Expr(std::forward<CONDITION>(condition)), body,
         ast::ElseStatementList{
-            std::forward<ElseStatements>(elseStatements)...});
+            std::forward<ELSE_STATEMENTS>(elseStatements)...});
   }
 
   /// Creates a ast::AssignmentStatement with input lhs and rhs expressions

src/writer/msl/generator_impl_array_accessor_test.cc

@@ -21,8 +21,10 @@ namespace {
 
 using MslGeneratorImplTest = TestHelper;
 
-TEST_F(MslGeneratorImplTest, EmitExpression_ArrayAccessor) {
+TEST_F(MslGeneratorImplTest, ArrayAccessor) {
-  auto* expr = IndexAccessor(Expr("ary"), 5);
+  auto* ary = Var("ary", ty.array<i32, 10>(), ast::StorageClass::kFunction);
+  auto* expr = IndexAccessor("ary", 5);
+  WrapInFunction(ary, expr);
 
   GeneratorImpl& gen = Build();
 
@@ -30,16 +32,6 @@ TEST_F(MslGeneratorImplTest, EmitExpression_ArrayAccessor) {
   EXPECT_EQ(gen.result(), "ary[5]");
 }
 
-TEST_F(MslGeneratorImplTest, EmitArrayAccessor) {
-  auto* expr = IndexAccessor(Expr("ary"), Expr("idx"));
-
-  GeneratorImpl& gen = Build();
-
-  ASSERT_TRUE(gen.EmitArrayAccessor(expr->As<ast::ArrayAccessorExpression>()))
-      << gen.error();
-  EXPECT_EQ(gen.result(), "ary[idx]");
-}
-
 }  // namespace
 }  // namespace msl
 }  // namespace writer

src/writer/msl/generator_impl_assign_test.cc

@@ -22,7 +22,10 @@ namespace {
 using MslGeneratorImplTest = TestHelper;
 
 TEST_F(MslGeneratorImplTest, Emit_Assign) {
+  auto* lhs = Var("lhs", ty.i32(), ast::StorageClass::kFunction);
+  auto* rhs = Var("rhs", ty.i32(), ast::StorageClass::kFunction);
   auto* assign = create<ast::AssignmentStatement>(Expr("lhs"), Expr("rhs"));
+  WrapInFunction(lhs, rhs, assign);
 
   GeneratorImpl& gen = Build();
 

src/writer/msl/generator_impl_binary_test.cc

@@ -31,8 +31,17 @@ using MslBinaryTest = TestParamHelper<BinaryData>;
 TEST_P(MslBinaryTest, Emit) {
   auto params = GetParam();
 
+  auto* type = ((params.op == ast::BinaryOp::kLogicalAnd) ||
+                (params.op == ast::BinaryOp::kLogicalOr))
+                   ? static_cast<type::Type*>(ty.bool_())
+                   : static_cast<type::Type*>(ty.u32());
+
+  auto* left = Var("left", type, ast::StorageClass::kFunction);
+  auto* right = Var("right", type, ast::StorageClass::kFunction);
+
   auto* expr =
       create<ast::BinaryExpression>(params.op, Expr("left"), Expr("right"));
+  WrapInFunction(left, right, expr);
 
   GeneratorImpl& gen = Build();
 

src/writer/msl/generator_impl_bitcast_test.cc

@@ -22,12 +22,13 @@ namespace {
 using MslGeneratorImplTest = TestHelper;
 
 TEST_F(MslGeneratorImplTest, EmitExpression_Bitcast) {
-  auto* bitcast = create<ast::BitcastExpression>(ty.f32(), Expr("id"));
+  auto* bitcast = create<ast::BitcastExpression>(ty.f32(), Expr(1));
+  WrapInFunction(bitcast);
 
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.EmitExpression(bitcast)) << gen.error();
-  EXPECT_EQ(gen.result(), "as_type<float>(id)");
+  EXPECT_EQ(gen.result(), "as_type<float>(1)");
 }
 
 }  // namespace

src/writer/msl/generator_impl_block_test.cc

@@ -22,9 +22,8 @@ namespace {
 using MslGeneratorImplTest = TestHelper;
 
 TEST_F(MslGeneratorImplTest, Emit_Block) {
-  auto* b = create<ast::BlockStatement>(ast::StatementList{
+  auto* b = Block(create<ast::DiscardStatement>());
-      create<ast::DiscardStatement>(),
+  WrapInFunction(b);
-  });
 
   GeneratorImpl& gen = Build();
 
@@ -38,9 +37,8 @@ TEST_F(MslGeneratorImplTest, Emit_Block) {
 }
 
 TEST_F(MslGeneratorImplTest, Emit_Block_WithoutNewline) {
-  auto* b = create<ast::BlockStatement>(ast::StatementList{
+  auto* b = Block(create<ast::DiscardStatement>());
-      create<ast::DiscardStatement>(),
+  WrapInFunction(b);
-  });
 
   GeneratorImpl& gen = Build();
 

src/writer/msl/generator_impl_break_test.cc

@@ -23,6 +23,7 @@ using MslGeneratorImplTest = TestHelper;
 
 TEST_F(MslGeneratorImplTest, Emit_Break) {
   auto* b = create<ast::BreakStatement>();
+  WrapInFunction(Loop(Block(b)));
 
   GeneratorImpl& gen = Build();
 

src/writer/msl/generator_impl_case_test.cc

@@ -23,12 +23,11 @@ namespace {
 using MslGeneratorImplTest = TestHelper;
 
 TEST_F(MslGeneratorImplTest, Emit_Case) {
-  auto* body = create<ast::BlockStatement>(ast::StatementList{
+  auto* body = Block(create<ast::BreakStatement>());
-      create<ast::BreakStatement>(),
-  });
   ast::CaseSelectorList lit;
-  lit.push_back(create<ast::SintLiteral>(ty.i32(), 5));
+  lit.push_back(Literal(5));
   auto* c = create<ast::CaseStatement>(lit, body);
+  WrapInFunction(c);
 
   GeneratorImpl& gen = Build();
 
@@ -43,9 +42,9 @@ TEST_F(MslGeneratorImplTest, Emit_Case) {
 
 TEST_F(MslGeneratorImplTest, Emit_Case_BreaksByDefault) {
   ast::CaseSelectorList lit;
-  lit.push_back(create<ast::SintLiteral>(ty.i32(), 5));
+  lit.push_back(Literal(5));
-  auto* c = create<ast::CaseStatement>(
+  auto* c = create<ast::CaseStatement>(lit, Block());
-      lit, create<ast::BlockStatement>(ast::StatementList{}));
+  WrapInFunction(c);
 
   GeneratorImpl& gen = Build();
 
@@ -59,12 +58,11 @@ TEST_F(MslGeneratorImplTest, Emit_Case_BreaksByDefault) {
 }
 
 TEST_F(MslGeneratorImplTest, Emit_Case_WithFallthrough) {
-  auto* body = create<ast::BlockStatement>(ast::StatementList{
+  auto* body = Block(create<ast::FallthroughStatement>());
-      create<ast::FallthroughStatement>(),
-  });
   ast::CaseSelectorList lit;
-  lit.push_back(create<ast::SintLiteral>(ty.i32(), 5));
+  lit.push_back(Literal(5));
   auto* c = create<ast::CaseStatement>(lit, body);
+  WrapInFunction(c);
 
   GeneratorImpl& gen = Build();
 
@@ -78,13 +76,12 @@ TEST_F(MslGeneratorImplTest, Emit_Case_WithFallthrough) {
 }
 
 TEST_F(MslGeneratorImplTest, Emit_Case_MultipleSelectors) {
-  auto* body = create<ast::BlockStatement>(ast::StatementList{
+  auto* body = Block(create<ast::BreakStatement>());
-      create<ast::BreakStatement>(),
-  });
   ast::CaseSelectorList lit;
-  lit.push_back(create<ast::SintLiteral>(ty.i32(), 5));
+  lit.push_back(Literal(5));
-  lit.push_back(create<ast::SintLiteral>(ty.i32(), 6));
+  lit.push_back(Literal(6));
   auto* c = create<ast::CaseStatement>(lit, body);
+  WrapInFunction(c);
 
   GeneratorImpl& gen = Build();
 
@@ -99,10 +96,9 @@ TEST_F(MslGeneratorImplTest, Emit_Case_MultipleSelectors) {
 }
 
 TEST_F(MslGeneratorImplTest, Emit_Case_Default) {
-  auto* body = create<ast::BlockStatement>(ast::StatementList{
+  auto* body = Block(create<ast::BreakStatement>());
-      create<ast::BreakStatement>(),
-  });
   auto* c = create<ast::CaseStatement>(ast::CaseSelectorList{}, body);
+  WrapInFunction(c);
 
   GeneratorImpl& gen = Build();
 

src/writer/msl/generator_impl_cast_test.cc

@@ -22,21 +22,23 @@ namespace {
 using MslGeneratorImplTest = TestHelper;
 
 TEST_F(MslGeneratorImplTest, EmitExpression_Cast_Scalar) {
-  auto* cast = Construct<f32>("id");
+  auto* cast = Construct<f32>(1);
+  WrapInFunction(cast);
 
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.EmitExpression(cast)) << gen.error();
-  EXPECT_EQ(gen.result(), "float(id)");
+  EXPECT_EQ(gen.result(), "float(1)");
 }
 
 TEST_F(MslGeneratorImplTest, EmitExpression_Cast_Vector) {
-  auto* cast = vec3<f32>("id");
+  auto* cast = vec3<f32>(vec3<i32>(1, 2, 3));
+  WrapInFunction(cast);
 
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.EmitExpression(cast)) << gen.error();
-  EXPECT_EQ(gen.result(), "float3(id)");
+  EXPECT_EQ(gen.result(), "float3(int3(1, 2, 3))");
 }
 
 }  // namespace

src/writer/msl/generator_impl_constructor_test.cc

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include "gmock/gmock.h"
 #include "src/writer/msl/test_helper.h"
 
 namespace tint {
@@ -19,163 +20,143 @@ namespace writer {
 namespace msl {
 namespace {
 
+using ::testing::HasSubstr;
+
 using MslGeneratorImplTest = TestHelper;
 
 TEST_F(MslGeneratorImplTest, EmitConstructor_Bool) {
-  auto* expr = Expr(false);
+  WrapInFunction(Expr(false));
 
   GeneratorImpl& gen = Build();
 
-  ASSERT_TRUE(gen.EmitConstructor(expr)) << gen.error();
+  ASSERT_TRUE(gen.Generate()) << gen.error();
-  EXPECT_EQ(gen.result(), "false");
+  EXPECT_THAT(gen.result(), HasSubstr("false"));
 }
 
 TEST_F(MslGeneratorImplTest, EmitConstructor_Int) {
-  auto* expr = Expr(-12345);
+  WrapInFunction(Expr(-12345));
 
   GeneratorImpl& gen = Build();
 
-  ASSERT_TRUE(gen.EmitConstructor(expr)) << gen.error();
+  ASSERT_TRUE(gen.Generate()) << gen.error();
-  EXPECT_EQ(gen.result(), "-12345");
+  EXPECT_THAT(gen.result(), HasSubstr("-12345"));
 }
 
 TEST_F(MslGeneratorImplTest, EmitConstructor_UInt) {
-  auto* expr = Expr(56779u);
+  WrapInFunction(Expr(56779u));
 
   GeneratorImpl& gen = Build();
 
-  ASSERT_TRUE(gen.EmitConstructor(expr)) << gen.error();
+  ASSERT_TRUE(gen.Generate()) << gen.error();
-  EXPECT_EQ(gen.result(), "56779u");
+  EXPECT_THAT(gen.result(), HasSubstr("56779u"));
 }
 
 TEST_F(MslGeneratorImplTest, EmitConstructor_Float) {
   // Use a number close to 1<<30 but whose decimal representation ends in 0.
-  auto* expr = Expr(static_cast<float>((1 << 30) - 4));
+  WrapInFunction(Expr(static_cast<float>((1 << 30) - 4)));
 
   GeneratorImpl& gen = Build();
 
-  ASSERT_TRUE(gen.EmitConstructor(expr)) << gen.error();
+  ASSERT_TRUE(gen.Generate()) << gen.error();
-  EXPECT_EQ(gen.result(), "1073741824.0f");
+  EXPECT_THAT(gen.result(), HasSubstr("1073741824.0f"));
 }
 
 TEST_F(MslGeneratorImplTest, EmitConstructor_Type_Float) {
-  auto* expr = Construct<f32>(-1.2e-5f);
+  WrapInFunction(Construct<f32>(-1.2e-5f));
 
   GeneratorImpl& gen = Build();
 
-  ASSERT_TRUE(gen.EmitConstructor(expr)) << gen.error();
+  ASSERT_TRUE(gen.Generate()) << gen.error();
-  EXPECT_EQ(gen.result(), "float(-0.000012f)");
+  EXPECT_THAT(gen.result(), HasSubstr("float(-0.000012f)"));
 }
 
 TEST_F(MslGeneratorImplTest, EmitConstructor_Type_Bool) {
-  auto* expr = Construct<bool>(true);
+  WrapInFunction(Construct<bool>(true));
 
   GeneratorImpl& gen = Build();
 
-  ASSERT_TRUE(gen.EmitConstructor(expr)) << gen.error();
+  ASSERT_TRUE(gen.Generate()) << gen.error();
-  EXPECT_EQ(gen.result(), "bool(true)");
+  EXPECT_THAT(gen.result(), HasSubstr("bool(true)"));
 }
 
 TEST_F(MslGeneratorImplTest, EmitConstructor_Type_Int) {
-  auto* expr = Construct<i32>(-12345);
+  WrapInFunction(Construct<i32>(-12345));
 
   GeneratorImpl& gen = Build();
 
-  ASSERT_TRUE(gen.EmitConstructor(expr)) << gen.error();
+  ASSERT_TRUE(gen.Generate()) << gen.error();
-  EXPECT_EQ(gen.result(), "int(-12345)");
+  EXPECT_THAT(gen.result(), HasSubstr("int(-12345)"));
 }
 
 TEST_F(MslGeneratorImplTest, EmitConstructor_Type_Uint) {
-  auto* expr = Construct<u32>(12345u);
+  WrapInFunction(Construct<u32>(12345u));
 
   GeneratorImpl& gen = Build();
 
-  ASSERT_TRUE(gen.EmitConstructor(expr)) << gen.error();
+  ASSERT_TRUE(gen.Generate()) << gen.error();
-  EXPECT_EQ(gen.result(), "uint(12345u)");
+  EXPECT_THAT(gen.result(), HasSubstr("uint(12345u)"));
 }
 
 TEST_F(MslGeneratorImplTest, EmitConstructor_Type_Vec) {
-  auto* expr = vec3<f32>(1.f, 2.f, 3.f);
+  WrapInFunction(vec3<f32>(1.f, 2.f, 3.f));
 
   GeneratorImpl& gen = Build();
 
-  ASSERT_TRUE(gen.EmitConstructor(expr)) << gen.error();
+  ASSERT_TRUE(gen.Generate()) << gen.error();
-  EXPECT_EQ(gen.result(), "float3(1.0f, 2.0f, 3.0f)");
+  EXPECT_THAT(gen.result(), HasSubstr("float3(1.0f, 2.0f, 3.0f)"));
 }
 
 TEST_F(MslGeneratorImplTest, EmitConstructor_Type_Vec_Empty) {
-  auto* expr = vec3<f32>();
+  WrapInFunction(vec3<f32>());
 
   GeneratorImpl& gen = Build();
 
-  ASSERT_TRUE(gen.EmitConstructor(expr)) << gen.error();
+  ASSERT_TRUE(gen.Generate()) << gen.error();
-  EXPECT_EQ(gen.result(), "float3(0.0f)");
+  EXPECT_THAT(gen.result(), HasSubstr("float3(0.0f)"));
 }
 
 TEST_F(MslGeneratorImplTest, EmitConstructor_Type_Mat) {
-  ast::ExpressionList mat_values;
+  WrapInFunction(Construct(ty.mat2x3<f32>(), vec3<f32>(1.0f, 2.0f, 3.0f),
-
+                           vec3<f32>(3.0f, 4.0f, 5.0f)));
-  for (size_t i = 0; i < 2; i++) {
-    mat_values.push_back(vec3<f32>(static_cast<float>(1 + (i * 2)),
-                                   static_cast<float>(2 + (i * 2)),
-                                   static_cast<float>(3 + (i * 2))));
-  }
-
-  auto* expr = Construct(ty.mat2x3<f32>(), mat_values);
 
   GeneratorImpl& gen = Build();
 
-  ASSERT_TRUE(gen.EmitConstructor(expr)) << gen.error();
+  ASSERT_TRUE(gen.Generate()) << gen.error();
 
   // A matrix of type T with n columns and m rows can also be constructed from
   // n vectors of type T with m components.
-  EXPECT_EQ(gen.result(),
+  EXPECT_THAT(
-            "float2x3(float3(1.0f, 2.0f, 3.0f), float3(3.0f, 4.0f, 5.0f))");
+      gen.result(),
+      HasSubstr(
+          "float2x3(float3(1.0f, 2.0f, 3.0f), float3(3.0f, 4.0f, 5.0f))"));
 }
 
 TEST_F(MslGeneratorImplTest, EmitConstructor_Type_Array) {
-  type::Array ary(ty.vec3<f32>(), 3, ast::DecorationList{});
+  WrapInFunction(
-
+      Construct(ty.array(ty.vec3<f32>(), 3), vec3<f32>(1.0f, 2.0f, 3.0f),
-  ast::ExpressionList ary_values;
+                vec3<f32>(4.0f, 5.0f, 6.0f), vec3<f32>(7.0f, 8.0f, 9.0f)));
-
-  for (size_t i = 0; i < 3; i++) {
-    ary_values.push_back(vec3<f32>(static_cast<float>(1 + (i * 3)),
-                                   static_cast<float>(2 + (i * 3)),
-                                   static_cast<float>(3 + (i * 3))));
-  }
-
-  auto* expr = Construct(&ary, ary_values);
 
   GeneratorImpl& gen = Build();
 
-  ASSERT_TRUE(gen.EmitConstructor(expr)) << gen.error();
+  ASSERT_TRUE(gen.Generate()) << gen.error();
-  EXPECT_EQ(gen.result(),
+  EXPECT_THAT(gen.result(),
-            "{float3(1.0f, 2.0f, 3.0f), float3(4.0f, 5.0f, 6.0f), "
+              HasSubstr("{float3(1.0f, 2.0f, 3.0f), float3(4.0f, 5.0f, 6.0f), "
-            "float3(7.0f, 8.0f, 9.0f)}");
+                        "float3(7.0f, 8.0f, 9.0f)}"));
 }
 
 TEST_F(MslGeneratorImplTest, EmitConstructor_Type_Struct) {
-  auto* struct_ty = Structure("S",
+  auto* str = Structure("S", {
-                              ast::StructMemberList{
+                                 Member("a", ty.i32()),
-                                  Member("a", ty.f32()),
+                                 Member("b", ty.f32()),
-                                  Member("b", ty.u32()),
+                                 Member("c", ty.vec3<i32>()),
-                                  Member("c", ty.vec4<f32>()),
+                             });
-                              },
-                              ast::DecorationList{});
 
-  ast::ExpressionList struct_values;
+  WrapInFunction(Construct(str, 1, 2.0f, vec3<i32>(3, 4, 5)));
-  struct_values.push_back(Expr(0.f));
-  struct_values.push_back(Expr(42u));
-  struct_values.push_back(Construct(
-      ty.vec4<f32>(),
-      ast::ExpressionList{Expr(1.f), Expr(2.f), Expr(3.f), Expr(4.f)}));
-
-  auto* expr = Construct(struct_ty, struct_values);
 
   GeneratorImpl& gen = Build();
 
-  ASSERT_TRUE(gen.EmitConstructor(expr)) << gen.error();
+  ASSERT_TRUE(gen.Generate()) << gen.error();
-  EXPECT_EQ(gen.result(), "{0.0f, 42u, float4(1.0f, 2.0f, 3.0f, 4.0f)}");
+  EXPECT_THAT(gen.result(), HasSubstr("{1, 2.0f, int3(3, 4, 5)}"));
 }
 
 }  // namespace

src/writer/msl/generator_impl_continue_test.cc

@@ -23,6 +23,7 @@ using MslGeneratorImplTest = TestHelper;
 
 TEST_F(MslGeneratorImplTest, Emit_Continue) {
   auto* c = create<ast::ContinueStatement>();
+  WrapInFunction(Loop(Block(c)));
 
   GeneratorImpl& gen = Build();
 

src/writer/msl/generator_impl_discard_test.cc

@@ -23,6 +23,7 @@ using MslGeneratorImplTest = TestHelper;
 
 TEST_F(MslGeneratorImplTest, Emit_Discard) {
   auto* stmt = create<ast::DiscardStatement>();
+  WrapInFunction(stmt);
 
   GeneratorImpl& gen = Build();
 

src/writer/msl/generator_impl_function_test.cc

@@ -577,9 +577,6 @@ TEST_F(MslGeneratorImplTest,
 
   Func("sub_func", params, ty.f32(), body, ast::DecorationList{});
 
-  ast::ExpressionList expr;
-  expr.push_back(Expr(1.0f));
-
   auto* var =
       Var("v", ty.f32(), ast::StorageClass::kFunction, Call("sub_func", 1.0f));
 
@@ -687,9 +684,6 @@ TEST_F(MslGeneratorImplTest,
 
   Func("sub_func", params, ty.f32(), body, ast::DecorationList{});
 
-  ast::ExpressionList expr;
-  expr.push_back(Expr(1.0f));
-
   auto* var =
       Var("v", ty.f32(), ast::StorageClass::kFunction, Call("sub_func", 1.0f));
 

src/writer/msl/generator_impl_identifier_test.cc

@@ -22,7 +22,10 @@ namespace {
 using MslGeneratorImplTest = TestHelper;
 
 TEST_F(MslGeneratorImplTest, EmitIdentifierExpression) {
+  auto* foo = Var("foo", ty.i32(), ast::StorageClass::kFunction);
+
   auto* i = Expr("foo");
+  WrapInFunction(foo, i);
 
   GeneratorImpl& gen = Build();
 

src/writer/msl/generator_impl_if_test.cc

@@ -22,11 +22,9 @@ namespace {
 using MslGeneratorImplTest = TestHelper;
 
 TEST_F(MslGeneratorImplTest, Emit_If) {
-  auto* cond = Expr("cond");
+  auto* cond = Var("cond", ty.bool_(), ast::StorageClass::kFunction);
-  auto* body = create<ast::BlockStatement>(ast::StatementList{
+  auto* i = If(cond, Block(Return()));
-      create<ast::ReturnStatement>(),
+  WrapInFunction(cond, i);
-  });
-  auto* i = create<ast::IfStatement>(cond, body, ast::ElseStatementList{});
 
   GeneratorImpl& gen = Build();
 
@@ -40,20 +38,10 @@ TEST_F(MslGeneratorImplTest, Emit_If) {
 }
 
 TEST_F(MslGeneratorImplTest, Emit_IfWithElseIf) {
-  auto* else_cond = Expr("else_cond");
+  auto* cond = Var("cond", ty.bool_(), ast::StorageClass::kFunction);
-  auto* else_body = create<ast::BlockStatement>(ast::StatementList{
+  auto* else_cond = Var("else_cond", ty.bool_(), ast::StorageClass::kFunction);
-      create<ast::ReturnStatement>(),
+  auto* i = If(cond, Block(Return()), Else(else_cond, Block(Return())));
-  });
+  WrapInFunction(cond, else_cond, i);
-
-  auto* cond = Expr("cond");
-  auto* body = create<ast::BlockStatement>(ast::StatementList{
-      create<ast::ReturnStatement>(),
-  });
-  auto* i = create<ast::IfStatement>(
-      cond, body,
-      ast::ElseStatementList{
-          create<ast::ElseStatement>(else_cond, else_body),
-      });
 
   GeneratorImpl& gen = Build();
 
@@ -69,19 +57,9 @@ TEST_F(MslGeneratorImplTest, Emit_IfWithElseIf) {
 }
 
 TEST_F(MslGeneratorImplTest, Emit_IfWithElse) {
-  auto* else_body = create<ast::BlockStatement>(ast::StatementList{
+  auto* cond = Var("cond", ty.bool_(), ast::StorageClass::kFunction);
-      create<ast::ReturnStatement>(),
+  auto* i = If(cond, Block(Return()), Else(nullptr, Block(Return())));
-  });
+  WrapInFunction(cond, i);
-
-  auto* cond = Expr("cond");
-  auto* body = create<ast::BlockStatement>(ast::StatementList{
-      create<ast::ReturnStatement>(),
-  });
-  auto* i = create<ast::IfStatement>(
-      cond, body,
-      ast::ElseStatementList{
-          create<ast::ElseStatement>(nullptr, else_body),
-      });
 
   GeneratorImpl& gen = Build();
 
@@ -97,26 +75,11 @@ TEST_F(MslGeneratorImplTest, Emit_IfWithElse) {
 }
 
 TEST_F(MslGeneratorImplTest, Emit_IfWithMultiple) {
-  auto* else_cond = Expr("else_cond");
+  auto* cond = Var("cond", ty.bool_(), ast::StorageClass::kFunction);
-
+  auto* else_cond = Var("else_cond", ty.bool_(), ast::StorageClass::kFunction);
-  auto* else_body = create<ast::BlockStatement>(ast::StatementList{
+  auto* i = If(cond, Block(Return()), Else(else_cond, Block(Return())),
-      create<ast::ReturnStatement>(),
+               Else(nullptr, Block(Return())));
-  });
+  WrapInFunction(cond, else_cond, i);
-
-  auto* else_body_2 = create<ast::BlockStatement>(ast::StatementList{
-      create<ast::ReturnStatement>(),
-  });
-
-  auto* cond = Expr("cond");
-  auto* body = create<ast::BlockStatement>(ast::StatementList{
-      create<ast::ReturnStatement>(),
-  });
-  auto* i = create<ast::IfStatement>(
-      cond, body,
-      ast::ElseStatementList{
-          create<ast::ElseStatement>(else_cond, else_body),
-          create<ast::ElseStatement>(nullptr, else_body_2),
-      });
 
   GeneratorImpl& gen = Build();
 

src/writer/msl/generator_impl_member_accessor_test.cc

@@ -22,12 +22,7 @@ namespace {
 using MslGeneratorImplTest = TestHelper;
 
 TEST_F(MslGeneratorImplTest, EmitExpression_MemberAccessor) {
-  Global("str",
+  Global("str", Structure("my_str", {Member("mem", ty.f32())}),
-         ty.struct_("my_str", create<ast::Struct>(
-                                  ast::StructMemberList{
-                                      Member("mem", ty.f32()),
-                                  },
-                                  ast::DecorationList{})),
          ast::StorageClass::kPrivate);
   auto* expr = MemberAccessor("str", "mem");
   WrapInFunction(expr);

src/writer/msl/generator_impl_return_test.cc

@@ -22,7 +22,8 @@ namespace {
 using MslGeneratorImplTest = TestHelper;
 
 TEST_F(MslGeneratorImplTest, Emit_Return) {
-  auto* r = create<ast::ReturnStatement>();
+  auto* r = Return();
+  WrapInFunction(r);
 
   GeneratorImpl& gen = Build();
 
@@ -33,14 +34,15 @@ TEST_F(MslGeneratorImplTest, Emit_Return) {
 }
 
 TEST_F(MslGeneratorImplTest, Emit_ReturnWithValue) {
-  auto* r = create<ast::ReturnStatement>(Expr("expr"));
+  auto* r = Return(123);
+  Func("f", {}, ty.i32(), {r});
 
   GeneratorImpl& gen = Build();
 
   gen.increment_indent();
 
   ASSERT_TRUE(gen.EmitStatement(r)) << gen.error();
-  EXPECT_EQ(gen.result(), "  return expr;\n");
+  EXPECT_EQ(gen.result(), "  return 123;\n");
 }
 
 }  // namespace

src/writer/msl/generator_impl_switch_test.cc

@@ -22,17 +22,15 @@ namespace {
 using MslGeneratorImplTest = TestHelper;
 
 TEST_F(MslGeneratorImplTest, Emit_Switch) {
-  auto* def_body = create<ast::BlockStatement>(ast::StatementList{
+  auto* cond = Var("cond", ty.i32(), ast::StorageClass::kFunction);
-      create<ast::BreakStatement>(),
+
-  });
+  auto* def_body = Block(create<ast::BreakStatement>());
   auto* def = create<ast::CaseStatement>(ast::CaseSelectorList{}, def_body);
 
   ast::CaseSelectorList case_val;
   case_val.push_back(Literal(5));
 
-  auto* case_body = create<ast::BlockStatement>(ast::StatementList{
+  auto* case_body = Block(create<ast::BreakStatement>());
-      create<ast::BreakStatement>(),
-  });
 
   auto* case_stmt = create<ast::CaseStatement>(case_val, case_body);
 
@@ -40,8 +38,8 @@ TEST_F(MslGeneratorImplTest, Emit_Switch) {
   body.push_back(case_stmt);
   body.push_back(def);
 
-  auto* s = create<ast::SwitchStatement>(Expr("cond"), body);
+  auto* s = create<ast::SwitchStatement>(Expr(cond), body);
-
+  WrapInFunction(cond, s);
   GeneratorImpl& gen = Build();
 
   gen.increment_indent();

src/writer/msl/generator_impl_unary_op_test.cc

@@ -30,7 +30,10 @@ inline std::ostream& operator<<(std::ostream& out, UnaryOpData data) {
 using MslUnaryOpTest = TestParamHelper<UnaryOpData>;
 TEST_P(MslUnaryOpTest, Emit) {
   auto params = GetParam();
+  Global("expr", ty.i32(), ast::StorageClass::kPrivate);
+
   auto* op = create<ast::UnaryOpExpression>(params.op, Expr("expr"));
+  WrapInFunction(op);
 
   GeneratorImpl& gen = Build();
 

src/writer/msl/generator_impl_variable_decl_statement_test.cc

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include "gmock/gmock.h"
 #include "src/ast/variable_decl_statement.h"
 #include "src/writer/msl/test_helper.h"
 
@@ -20,6 +21,8 @@ namespace writer {
 namespace msl {
 namespace {
 
+using ::testing::HasSubstr;
+
 using MslGeneratorImplTest = TestHelper;
 
 TEST_F(MslGeneratorImplTest, Emit_VariableDeclStatement) {
@@ -109,29 +112,34 @@ TEST_F(MslGeneratorImplTest, Emit_VariableDeclStatement_Matrix) {
   EXPECT_EQ(gen.result(), "  float3x2 a = 0.0f;\n");
 }
 
-TEST_F(MslGeneratorImplTest, Emit_VariableDeclStatement_Private) {
+// TODO(crbug.com/tint/726): module-scope private and workgroup variables not
-  auto* var = Global("a", ty.f32(), ast::StorageClass::kPrivate);
+// yet implemented
-  auto* stmt = create<ast::VariableDeclStatement>(var);
+TEST_F(MslGeneratorImplTest, DISABLED_Emit_VariableDeclStatement_Private) {
+  Global("a", ty.f32(), ast::StorageClass::kPrivate);
+
+  WrapInFunction(Expr("a"));
 
   GeneratorImpl& gen = Build();
 
   gen.increment_indent();
 
-  ASSERT_TRUE(gen.EmitStatement(stmt)) << gen.error();
+  ASSERT_TRUE(gen.Generate()) << gen.error();
-  EXPECT_EQ(gen.result(), "  float a = 0.0f;\n");
+  EXPECT_THAT(gen.result(), HasSubstr("  float a = 0.0f;\n"));
 }
 
-TEST_F(MslGeneratorImplTest, Emit_VariableDeclStatement_Initializer_Private) {
+// TODO(crbug.com/tint/726): module-scope private and workgroup variables not
+// yet implemented
+TEST_F(MslGeneratorImplTest,
+       DISABLED_Emit_VariableDeclStatement_Initializer_Private) {
   Global("initializer", ty.f32(), ast::StorageClass::kInput);
-  auto* var =
   Global("a", ty.f32(), ast::StorageClass::kPrivate, Expr("initializer"));
-  auto* stmt = create<ast::VariableDeclStatement>(var);
+
+  WrapInFunction(Expr("a"));
 
   GeneratorImpl& gen = Build();
 
-  ASSERT_TRUE(gen.EmitStatement(stmt)) << gen.error();
+  ASSERT_TRUE(gen.Generate()) << gen.error();
-  EXPECT_EQ(gen.result(), R"(float a = initializer;
+  EXPECT_THAT(gen.result(), HasSubstr("float a = initializer;\n"));
-)");
 }
 
 TEST_F(MslGeneratorImplTest, Emit_VariableDeclStatement_Initializer_ZeroVec) {