resolver: Validate unreachable stmts when terminator is in block(s)

Will requires updating the WGSL spec, which currently has rules looser than SPIR-V.

Fixed: tint:1167
Bug: chromium:1246163
Change-Id: Ie8fcfabc0bb89c7fb69c345475ff99c07fa04172
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/63560
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>

src/resolver/control_block_validation_test.cc

@@ -111,21 +111,39 @@ TEST_F(ResolverControlBlockValidationTest, SwitchWithTwoDefault_Fail) {
 
 TEST_F(ResolverControlBlockValidationTest, UnreachableCode_Loop_continue) {
   // loop {
-  //   continue;
   //   var z: i32;
+  //   continue;
+  //   z = 1;
   // }
-  WrapInFunction(Loop(Block(
+  WrapInFunction(Loop(Block(Decl(Var("z", ty.i32(), ast::StorageClass::kNone)),
                             create<ast::ContinueStatement>(),
-      Decl(Source{{12, 34}}, Var("z", ty.i32(), ast::StorageClass::kNone)))));
+                            Assign(Source{{12, 34}}, "z", 1))));
+
+  EXPECT_FALSE(r()->Resolve()) << r()->error();
+  EXPECT_EQ(r()->error(), "12:34 error: code is unreachable");
+}
+
+TEST_F(ResolverControlBlockValidationTest,
+       UnreachableCode_Loop_continue_InBlocks) {
+  // loop {
+  //   var z: i32;
+  //   {{{continue;}}}
+  //   z = 1;
+  // }
+  WrapInFunction(
+      Loop(Block(Decl(Var("z", ty.i32(), ast::StorageClass::kNone)),
+                 Block(Block(Block(create<ast::ContinueStatement>()))),
+                 Assign(Source{{12, 34}}, "z", 1))));
 
   EXPECT_FALSE(r()->Resolve()) << r()->error();
   EXPECT_EQ(r()->error(), "12:34 error: code is unreachable");
 }
 
 TEST_F(ResolverControlBlockValidationTest, UnreachableCode_ForLoop_continue) {
-  // for (;;;) {
+  // for (;;) {
-  //   continue;
   //   var z: i32;
+  //   continue;
+  //   z = 1;
   // }
   WrapInFunction(
       For(nullptr, nullptr, nullptr,
@@ -136,6 +154,24 @@ TEST_F(ResolverControlBlockValidationTest, UnreachableCode_ForLoop_continue) {
   EXPECT_FALSE(r()->Resolve()) << r()->error();
   EXPECT_EQ(r()->error(), "12:34 error: code is unreachable");
 }
+
+TEST_F(ResolverControlBlockValidationTest,
+       UnreachableCode_ForLoop_continue_InBlocks) {
+  // for (;;) {
+  //   var z: i32;
+  //   {{{continue;}}}
+  //   z = 1;
+  // }
+  WrapInFunction(
+      For(nullptr, nullptr, nullptr,
+          Block(Decl(Var("z", ty.i32(), ast::StorageClass::kNone)),
+                Block(Block(Block(create<ast::ContinueStatement>()))),
+                Assign(Source{{12, 34}}, "z", 1))));
+
+  EXPECT_FALSE(r()->Resolve()) << r()->error();
+  EXPECT_EQ(r()->error(), "12:34 error: code is unreachable");
+}
+
 TEST_F(ResolverControlBlockValidationTest, UnreachableCode_break) {
   // switch (a) {
   //   case 1: { break; var a : u32 = 2;}
@@ -152,6 +188,24 @@ TEST_F(ResolverControlBlockValidationTest, UnreachableCode_break) {
   EXPECT_EQ(r()->error(), "12:34 error: code is unreachable");
 }
 
+TEST_F(ResolverControlBlockValidationTest, UnreachableCode_break_InBlocks) {
+  // switch (a) {
+  //   case 1: { {{{break;}}} var a : u32 = 2;}
+  //   default: {}
+  // }
+  auto* decl = Decl(Source{{12, 34}},
+                    Var("a", ty.i32(), ast::StorageClass::kNone, Expr(2)));
+
+  WrapInFunction(Loop(Block(Switch(
+      Expr(1),
+      Case(Literal(1),
+           Block(Block(Block(Block(create<ast::BreakStatement>()))), decl)),
+      DefaultCase()))));
+
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(r()->error(), "12:34 error: code is unreachable");
+}
+
 TEST_F(ResolverControlBlockValidationTest,
        SwitchConditionTypeMustMatchSelectorType2_Fail) {
   // var a : u32 = 2;

src/resolver/function_validation_test.cc

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include "src/ast/discard_statement.h"
 #include "src/ast/return_statement.h"
 #include "src/ast/stage_decoration.h"
 #include "src/resolver/resolver.h"
@@ -172,18 +173,68 @@ TEST_F(ResolverFunctionValidationTest,
 
 TEST_F(ResolverFunctionValidationTest, UnreachableCode_return) {
   // fn func() -> {
+  //  var a : i32;
   //  return;
-  //  var a: i32 = 2;
+  //  a = 2;
   //}
-  auto* decl = Decl(Source{{12, 34}},
-                    Var("a", ty.i32(), ast::StorageClass::kNone, Expr(2)));
 
   Func("func", ast::VariableList{}, ty.void_(),
-       ast::StatementList{
+       {
+           Decl(Var("a", ty.i32(), ast::StorageClass::kNone, Expr(2))),
            Return(),
-           decl,
+           Assign(Source{{12, 34}}, "a", 2),
-       },
+       });
-       ast::DecorationList{});
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(r()->error(), "12:34 error: code is unreachable");
+}
+
+TEST_F(ResolverFunctionValidationTest, UnreachableCode_return_InBlocks) {
+  // fn func() -> {
+  //  var a : i32;
+  //  {{{return;}}}
+  //  a = 2;
+  //}
+
+  Func("func", ast::VariableList{}, ty.void_(),
+       {
+           Decl(Var("a", ty.i32(), ast::StorageClass::kNone, Expr(2))),
+           Block(Block(Block(Return()))),
+           Assign(Source{{12, 34}}, "a", 2),
+       });
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(r()->error(), "12:34 error: code is unreachable");
+}
+
+TEST_F(ResolverFunctionValidationTest, UnreachableCode_discard) {
+  // fn func() -> {
+  //  var a : i32;
+  //  discard;
+  //  a = 2;
+  //}
+
+  Func("func", ast::VariableList{}, ty.void_(),
+       {
+           Decl(Var("a", ty.i32(), ast::StorageClass::kNone, Expr(2))),
+           create<ast::DiscardStatement>(),
+           Assign(Source{{12, 34}}, "a", 2),
+       });
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(r()->error(), "12:34 error: code is unreachable");
+}
+
+TEST_F(ResolverFunctionValidationTest, UnreachableCode_discard_InBlocks) {
+  // fn func() -> {
+  //  var a : i32;
+  //  {{{discard;}}}
+  //  a = 2;
+  //}
+
+  Func("func", ast::VariableList{}, ty.void_(),
+       {
+           Decl(Var("a", ty.i32(), ast::StorageClass::kNone, Expr(2))),
+           Block(Block(Block(create<ast::DiscardStatement>()))),
+           Assign(Source{{12, 34}}, "a", 2),
+       });
   EXPECT_FALSE(r()->Resolve());
   EXPECT_EQ(r()->error(), "12:34 error: code is unreachable");
 }

src/resolver/resolver.cc

@@ -1964,15 +1964,23 @@ bool Resolver::Statements(const ast::StatementList& stmts) {
 }
 
 bool Resolver::ValidateStatements(const ast::StatementList& stmts) {
-  auto next_stmt = stmts.begin();
+  bool unreachable = false;
   for (auto* stmt : stmts) {
-    next_stmt++;
+    if (unreachable) {
-    if (stmt->IsAnyOf<ast::ReturnStatement, ast::BreakStatement,
+      AddError("code is unreachable", stmt->source());
-                      ast::ContinueStatement>()) {
-      if (stmt != stmts.back()) {
-        AddError("code is unreachable", (*next_stmt)->source());
       return false;
     }
+
+    auto* nested_stmt = stmt;
+    while (auto* block = nested_stmt->As<ast::BlockStatement>()) {
+      if (block->empty()) {
+        break;
+      }
+      nested_stmt = block->statements().back();
+    }
+    if (nested_stmt->IsAnyOf<ast::ReturnStatement, ast::BreakStatement,
+                             ast::ContinueStatement, ast::DiscardStatement>()) {
+      unreachable = true;
     }
   }
   return true;

src/resolver/validation_test.cc

@@ -476,6 +476,32 @@ TEST_F(ResolverValidationTest,
   EXPECT_EQ(r()->error(), "12:34 error: code is unreachable");
 }
 
+TEST_F(
+    ResolverValidationTest,
+    Stmt_Loop_ContinueInLoopBodyBeforeDeclAndAfterDecl_UsageInContinuing_InBlocks) {  // NOLINT - line length
+  // loop  {
+  //     var z : i32;
+  //     {{{continue;}}} // Bypasses z decl
+  //     z = 1;
+  //     continue; // Ok
+  //
+  //     continuing {
+  //         z = 2;
+  //     }
+  // }
+
+  auto* body =
+      Block(Decl(Var("z", ty.i32(), ast::StorageClass::kNone)),
+            Block(Block(Block(create<ast::ContinueStatement>()))),
+            Assign(Source{{12, 34}}, "z", 2), create<ast::ContinueStatement>());
+  auto* continuing = Block(Assign(Expr("z"), 2));
+  auto* loop_stmt = Loop(body, continuing);
+  WrapInFunction(loop_stmt);
+
+  EXPECT_FALSE(r()->Resolve()) << r()->error();
+  EXPECT_EQ(r()->error(), "12:34 error: code is unreachable");
+}
+
 TEST_F(ResolverValidationTest,
        Stmt_Loop_ContinueInLoopBodySubscopeBeforeDecl_UsageInContinuing) {
   // loop  {

src/writer/wgsl/generator_impl_function_test.cc

@@ -28,7 +28,6 @@ using WgslGeneratorImplTest = TestHelper;
 TEST_F(WgslGeneratorImplTest, Emit_Function) {
   auto* func = Func("my_func", ast::VariableList{}, ty.void_(),
                     ast::StatementList{
-                        create<ast::DiscardStatement>(),
                         Return(),
                     },
                     ast::DecorationList{});
@@ -39,7 +38,6 @@ TEST_F(WgslGeneratorImplTest, Emit_Function) {
 
   ASSERT_TRUE(gen.EmitFunction(func));
   EXPECT_EQ(gen.result(), R"(  fn my_func() {
-    discard;
     return;
   }
 )");
@@ -50,7 +48,6 @@ TEST_F(WgslGeneratorImplTest, Emit_Function_WithParams) {
       "my_func", ast::VariableList{Param("a", ty.f32()), Param("b", ty.i32())},
       ty.void_(),
       ast::StatementList{
-          create<ast::DiscardStatement>(),
           Return(),
       },
       ast::DecorationList{});
@@ -61,7 +58,6 @@ TEST_F(WgslGeneratorImplTest, Emit_Function_WithParams) {
 
   ASSERT_TRUE(gen.EmitFunction(func));
   EXPECT_EQ(gen.result(), R"(  fn my_func(a : f32, b : i32) {
-    discard;
     return;
   }
 )");