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>
diff --git a/src/resolver/control_block_validation_test.cc b/src/resolver/control_block_validation_test.cc
index bc8fc39..6ee6c55 100644
--- a/src/resolver/control_block_validation_test.cc
+++ b/src/resolver/control_block_validation_test.cc
@@ -111,21 +111,39 @@
TEST_F(ResolverControlBlockValidationTest, UnreachableCode_Loop_continue) {
// loop {
+ // var z: i32;
// continue;
- // var z : i32;
+ // z = 1;
// }
- WrapInFunction(Loop(Block(
- create<ast::ContinueStatement>(),
- Decl(Source{{12, 34}}, Var("z", ty.i32(), ast::StorageClass::kNone)))));
+ WrapInFunction(Loop(Block(Decl(Var("z", ty.i32(), ast::StorageClass::kNone)),
+ 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_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 (;;) {
+ // var z: i32;
// continue;
- // var z : i32;
+ // z = 1;
// }
WrapInFunction(
For(nullptr, nullptr, nullptr,
@@ -136,6 +154,24 @@
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 @@
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;
diff --git a/src/resolver/function_validation_test.cc b/src/resolver/function_validation_test.cc
index 4e87a8c..50a8e21 100644
--- a/src/resolver/function_validation_test.cc
+++ b/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, 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,
- },
- ast::DecorationList{});
+ Assign(Source{{12, 34}}, "a", 2),
+ });
+ 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");
}
diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index f7c958b..1a1143f 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -1964,15 +1964,23 @@
}
bool Resolver::ValidateStatements(const ast::StatementList& stmts) {
- auto next_stmt = stmts.begin();
+ bool unreachable = false;
for (auto* stmt : stmts) {
- next_stmt++;
- if (stmt->IsAnyOf<ast::ReturnStatement, ast::BreakStatement,
- ast::ContinueStatement>()) {
- if (stmt != stmts.back()) {
- AddError("code is unreachable", (*next_stmt)->source());
- return false;
+ if (unreachable) {
+ AddError("code is unreachable", 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;
diff --git a/src/resolver/validation_test.cc b/src/resolver/validation_test.cc
index bad2a8e..aea2321 100644
--- a/src/resolver/validation_test.cc
+++ b/src/resolver/validation_test.cc
@@ -476,6 +476,32 @@
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 {
diff --git a/src/writer/wgsl/generator_impl_function_test.cc b/src/writer/wgsl/generator_impl_function_test.cc
index 62e0606..89dd681 100644
--- a/src/writer/wgsl/generator_impl_function_test.cc
+++ b/src/writer/wgsl/generator_impl_function_test.cc
@@ -28,7 +28,6 @@
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 @@
ASSERT_TRUE(gen.EmitFunction(func));
EXPECT_EQ(gen.result(), R"( fn my_func() {
- discard;
return;
}
)");
@@ -50,7 +48,6 @@
"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 @@
ASSERT_TRUE(gen.EmitFunction(func));
EXPECT_EQ(gen.result(), R"( fn my_func(a : f32, b : i32) {
- discard;
return;
}
)");
