Resolver: Validation for continuing blocks
Check they do not contain returns, discards
Check they do not directly contain continues, however a nested loop can have its own continue.
Bug: chromium:1229976
Change-Id: Ia3c4ac118ffdaa6cca6025366c19f9897718c930
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/58384
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: David Neto <dneto@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>
diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index 8288a3f..ca5ea27 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -2062,11 +2062,18 @@
}
if (stmt->Is<ast::ContinueStatement>()) {
// Set if we've hit the first continue statement in our parent loop
- if (auto* loop_block =
- current_block_->FindFirstParent<sem::LoopBlockStatement>()) {
- if (loop_block->FirstContinue() == size_t(~0)) {
- const_cast<sem::LoopBlockStatement*>(loop_block)
- ->SetFirstContinue(loop_block->Decls().size());
+ if (auto* block =
+ current_block_->FindFirstParent<
+ sem::LoopBlockStatement, sem::LoopContinuingBlockStatement>()) {
+ if (auto* loop_block = block->As<sem::LoopBlockStatement>()) {
+ if (loop_block->FirstContinue() == size_t(~0)) {
+ const_cast<sem::LoopBlockStatement*>(loop_block)
+ ->SetFirstContinue(loop_block->Decls().size());
+ }
+ } else {
+ AddError("continuing blocks must not contain a continue statement",
+ stmt->source());
+ return false;
}
} else {
AddError("continue statement must be in a loop", stmt->source());
@@ -2076,6 +2083,17 @@
return true;
}
if (stmt->Is<ast::DiscardStatement>()) {
+ if (auto* continuing =
+ sem_statement
+ ->FindFirstParent<sem::LoopContinuingBlockStatement>()) {
+ AddError("continuing blocks must not contain a discard statement",
+ stmt->source());
+ if (continuing != sem_statement->Parent()) {
+ AddNote("see continuing block here",
+ continuing->Declaration()->source());
+ }
+ return false;
+ }
return true;
}
if (stmt->Is<ast::FallthroughStatement>()) {
@@ -4110,6 +4128,17 @@
return false;
}
+ auto* sem = builder_->Sem().Get(ret);
+ if (auto* continuing =
+ sem->FindFirstParent<sem::LoopContinuingBlockStatement>()) {
+ AddError("continuing blocks must not contain a return statement",
+ ret->source());
+ if (continuing != sem->Parent()) {
+ AddNote("see continuing block here", continuing->Declaration()->source());
+ }
+ return false;
+ }
+
return true;
}
diff --git a/src/resolver/validation_test.cc b/src/resolver/validation_test.cc
index 573629a..6bb426f 100644
--- a/src/resolver/validation_test.cc
+++ b/src/resolver/validation_test.cc
@@ -20,6 +20,7 @@
#include "src/ast/break_statement.h"
#include "src/ast/call_statement.h"
#include "src/ast/continue_statement.h"
+#include "src/ast/discard_statement.h"
#include "src/ast/if_statement.h"
#include "src/ast/intrinsic_texture_helper_test.h"
#include "src/ast/loop_statement.h"
@@ -650,6 +651,123 @@
EXPECT_TRUE(r()->Resolve());
}
+TEST_F(ResolverTest, Stmt_Loop_ReturnInContinuing_Direct) {
+ // loop {
+ // continuing {
+ // return;
+ // }
+ // }
+
+ WrapInFunction(Loop( // loop
+ Block(), // loop block
+ Block( // loop continuing block
+ Return(Source{{12, 34}}))));
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(
+ r()->error(),
+ R"(12:34 error: continuing blocks must not contain a return statement)");
+}
+
+TEST_F(ResolverTest, Stmt_Loop_ReturnInContinuing_Indirect) {
+ // loop {
+ // continuing {
+ // loop {
+ // return;
+ // }
+ // }
+ // }
+
+ WrapInFunction(Loop( // outer loop
+ Block(), // outer loop block
+ Block(Source{{56, 78}}, // outer loop continuing block
+ Loop( // inner loop
+ Block( // inner loop block
+ Return(Source{{12, 34}}))))));
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(
+ r()->error(),
+ R"(12:34 error: continuing blocks must not contain a return statement
+56:78 note: see continuing block here)");
+}
+
+TEST_F(ResolverTest, Stmt_Loop_DiscardInContinuing_Direct) {
+ // loop {
+ // continuing {
+ // discard;
+ // }
+ // }
+
+ WrapInFunction(Loop( // loop
+ Block(), // loop block
+ Block( // loop continuing block
+ create<ast::DiscardStatement>(Source{{12, 34}}))));
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(
+ r()->error(),
+ R"(12:34 error: continuing blocks must not contain a discard statement)");
+}
+
+TEST_F(ResolverTest, Stmt_Loop_DiscardInContinuing_Indirect) {
+ // loop {
+ // continuing {
+ // loop { discard; }
+ // }
+ // }
+
+ WrapInFunction(Loop( // outer loop
+ Block(), // outer loop block
+ Block(Source{{56, 78}}, // outer loop continuing block
+ Loop( // inner loop
+ Block( // inner loop block
+ create<ast::DiscardStatement>(Source{{12, 34}}))))));
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(
+ r()->error(),
+ R"(12:34 error: continuing blocks must not contain a discard statement
+56:78 note: see continuing block here)");
+}
+
+TEST_F(ResolverTest, Stmt_Loop_ContinueInContinuing_Direct) {
+ // loop {
+ // continuing {
+ // continue;
+ // }
+ // }
+
+ WrapInFunction(Loop( // loop
+ Block(), // loop block
+ Block(Source{{56, 78}}, // loop continuing block
+ create<ast::ContinueStatement>(Source{{12, 34}}))));
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(
+ r()->error(),
+ "12:34 error: continuing blocks must not contain a continue statement");
+}
+
+TEST_F(ResolverTest, Stmt_Loop_ContinueInContinuing_Indirect) {
+ // loop {
+ // continuing {
+ // loop {
+ // continue;
+ // }
+ // }
+ // }
+
+ WrapInFunction(Loop( // outer loop
+ Block(), // outer loop block
+ Block( // outer loop continuing block
+ Loop( // inner loop
+ Block( // inner loop block
+ create<ast::ContinueStatement>(Source{{12, 34}}))))));
+
+ EXPECT_TRUE(r()->Resolve()) << r()->error();
+}
+
TEST_F(ResolverTest, Stmt_ForLoop_CondIsNotBool) {
// for (; 1.0f; ) {
// }
