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>
This commit is contained in:
Ben Clayton
2021-07-17 18:09:26 +00:00
committed by Tint LUCI CQ
parent 725159c17c
commit cd7eb4f968
7 changed files with 335 additions and 71 deletions

View File

@@ -2062,11 +2062,18 @@ bool Resolver::Statement(ast::Statement* stmt) {
}
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 @@ bool Resolver::Statement(ast::Statement* stmt) {
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 @@ bool Resolver::ValidateReturn(const ast::ReturnStatement* ret) {
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;
}

View File

@@ -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 @@ TEST_F(ResolverTest, Stmt_Loop_ContinueInLoopBodyAfterDecl_UsageInContinuing) {
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; ) {
// }