validation: Error on obviously infinite loops

Most of this change is fixing up the numerious tests that violated this rule.

Fixed: tint:1365
Issue: tint:1374
Change-Id: I38da27c7367277fe60857208170fec017e80bd25
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/76400
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
This commit is contained in:
Ben Clayton
2022-01-14 17:16:32 +00:00
committed by Tint LUCI CQ
parent 8dd9a56f91
commit e5919ac115
233 changed files with 3562 additions and 1422 deletions

View File

@@ -82,15 +82,15 @@ TEST_F(ResolverCompoundStatementTest, Block) {
TEST_F(ResolverCompoundStatementTest, Loop) {
// fn F() {
// loop {
// stmt_a;
// break;
// continuing {
// stmt_b;
// stmt;
// }
// }
// }
auto* stmt_a = Ignore(1);
auto* stmt_b = Ignore(1);
auto* loop = Loop(Block(stmt_a), Block(stmt_b));
auto* brk = Break();
auto* stmt = Ignore(1);
auto* loop = Loop(Block(brk), Block(stmt));
auto* f = Func("F", {}, ty.void_(), {loop});
ASSERT_TRUE(r()->Resolve()) << r()->error();
@@ -103,7 +103,7 @@ TEST_F(ResolverCompoundStatementTest, Loop) {
EXPECT_EQ(s->Parent(), s->Block());
}
{
auto* s = Sem().Get(stmt_a);
auto* s = Sem().Get(brk);
ASSERT_NE(s, nullptr);
ASSERT_NE(s->Block(), nullptr);
EXPECT_EQ(s->Parent(), s->Block());
@@ -122,7 +122,7 @@ TEST_F(ResolverCompoundStatementTest, Loop) {
EXPECT_EQ(s->Parent()->Parent()->Parent()->Parent(), nullptr);
}
{
auto* s = Sem().Get(stmt_b);
auto* s = Sem().Get(stmt);
ASSERT_NE(s, nullptr);
ASSERT_NE(s->Block(), nullptr);
EXPECT_EQ(s->Parent(), s->Block());

View File

@@ -86,6 +86,7 @@ TEST_F(ResolverControlBlockValidationTest, SwitchWithTwoDefault_Fail) {
TEST_F(ResolverControlBlockValidationTest, UnreachableCode_Loop_continue) {
// loop {
// if (false) { break; }
// var z: i32;
// continue;
// z = 1;
@@ -93,7 +94,8 @@ TEST_F(ResolverControlBlockValidationTest, UnreachableCode_Loop_continue) {
auto* decl_z = Decl(Var("z", ty.i32()));
auto* cont = Continue();
auto* assign_z = Assign(Source{{12, 34}}, "z", 1);
WrapInFunction(Loop(Block(decl_z, cont, assign_z)));
WrapInFunction(
Loop(Block(If(false, Block(Break())), decl_z, cont, assign_z)));
ASSERT_TRUE(r()->Resolve()) << r()->error();
EXPECT_EQ(r()->error(), "12:34 warning: code is unreachable");
@@ -105,6 +107,7 @@ TEST_F(ResolverControlBlockValidationTest, UnreachableCode_Loop_continue) {
TEST_F(ResolverControlBlockValidationTest,
UnreachableCode_Loop_continue_InBlocks) {
// loop {
// if (false) { break; }
// var z: i32;
// {{{continue;}}}
// z = 1;
@@ -112,7 +115,8 @@ TEST_F(ResolverControlBlockValidationTest,
auto* decl_z = Decl(Var("z", ty.i32()));
auto* cont = Continue();
auto* assign_z = Assign(Source{{12, 34}}, "z", 1);
WrapInFunction(Loop(Block(decl_z, Block(Block(Block(cont))), assign_z)));
WrapInFunction(Loop(Block(If(false, Block(Break())), decl_z,
Block(Block(Block(cont))), assign_z)));
ASSERT_TRUE(r()->Resolve()) << r()->error();
EXPECT_EQ(r()->error(), "12:34 warning: code is unreachable");
@@ -122,7 +126,7 @@ TEST_F(ResolverControlBlockValidationTest,
}
TEST_F(ResolverControlBlockValidationTest, UnreachableCode_ForLoop_continue) {
// for (;;) {
// for (;false;) {
// var z: i32;
// continue;
// z = 1;
@@ -130,7 +134,7 @@ TEST_F(ResolverControlBlockValidationTest, UnreachableCode_ForLoop_continue) {
auto* decl_z = Decl(Var("z", ty.i32()));
auto* cont = Continue();
auto* assign_z = Assign(Source{{12, 34}}, "z", 1);
WrapInFunction(For(nullptr, nullptr, nullptr, //
WrapInFunction(For(nullptr, false, nullptr, //
Block(decl_z, cont, assign_z)));
ASSERT_TRUE(r()->Resolve()) << r()->error();
@@ -142,7 +146,7 @@ TEST_F(ResolverControlBlockValidationTest, UnreachableCode_ForLoop_continue) {
TEST_F(ResolverControlBlockValidationTest,
UnreachableCode_ForLoop_continue_InBlocks) {
// for (;;) {
// for (;false;) {
// var z: i32;
// {{{continue;}}}
// z = 1;
@@ -150,7 +154,7 @@ TEST_F(ResolverControlBlockValidationTest,
auto* decl_z = Decl(Var("z", ty.i32()));
auto* cont = Continue();
auto* assign_z = Assign(Source{{12, 34}}, "z", 1);
WrapInFunction(For(nullptr, nullptr, nullptr,
WrapInFunction(For(nullptr, false, nullptr,
Block(decl_z, Block(Block(Block(cont))), assign_z)));
ASSERT_TRUE(r()->Resolve()) << r()->error();
@@ -171,10 +175,10 @@ TEST_F(ResolverControlBlockValidationTest, UnreachableCode_break) {
auto* decl_z = Decl(Var("z", ty.i32()));
auto* brk = Break();
auto* assign_z = Assign(Source{{12, 34}}, "z", 1);
WrapInFunction( //
Loop(Block(Switch(1, //
Case(Expr(1), Block(decl_z, brk, assign_z)), //
DefaultCase()))));
WrapInFunction( //
Block(Switch(1, //
Case(Expr(1), Block(decl_z, brk, assign_z)), //
DefaultCase())));
ASSERT_TRUE(r()->Resolve()) << r()->error();
EXPECT_EQ(r()->error(), "12:34 warning: code is unreachable");
@@ -189,6 +193,7 @@ TEST_F(ResolverControlBlockValidationTest, UnreachableCode_break_InBlocks) {
// case 1: { {{{break;}}} var a : u32 = 2;}
// default: {}
// }
// break;
// }
auto* decl_z = Decl(Var("z", ty.i32()));
auto* brk = Break();
@@ -196,7 +201,8 @@ TEST_F(ResolverControlBlockValidationTest, UnreachableCode_break_InBlocks) {
WrapInFunction(Loop(Block(
Switch(1, //
Case(Expr(1), Block(decl_z, Block(Block(Block(brk))), assign_z)),
DefaultCase()))));
DefaultCase()), //
Break())));
ASSERT_TRUE(r()->Resolve()) << r()->error();
EXPECT_EQ(r()->error(), "12:34 warning: code is unreachable");

View File

@@ -1039,7 +1039,7 @@ sem::LoopStatement* Resolver::LoopStatement(const ast::LoopStatement* stmt) {
}
behaviors.Remove(sem::Behavior::kBreak, sem::Behavior::kContinue);
return true;
return ValidateLoopStatement(sem);
});
});
}

View File

@@ -264,6 +264,7 @@ class Resolver {
std::unordered_set<uint32_t>& locations,
const Source& source,
const bool is_input = false);
bool ValidateLoopStatement(const sem::LoopStatement* stmt);
bool ValidateMatrix(const sem::Matrix* ty, const Source& source);
bool ValidateFunctionParameter(const ast::Function* func,
const sem::Variable* var);

View File

@@ -216,7 +216,8 @@ TEST_F(ResolverBehaviorTest, StmtBreak) {
TEST_F(ResolverBehaviorTest, StmtContinue) {
auto* stmt = Continue();
WrapInFunction(Loop(Block(stmt)));
WrapInFunction(Loop(Block(If(true, Block(Break())), //
stmt)));
ASSERT_TRUE(r()->Resolve()) << r()->error();
@@ -234,14 +235,12 @@ TEST_F(ResolverBehaviorTest, StmtDiscard) {
EXPECT_EQ(sem->Behaviors(), sem::Behavior::kDiscard);
}
TEST_F(ResolverBehaviorTest, StmtForLoopEmpty) {
auto* stmt = For(nullptr, nullptr, nullptr, Block());
TEST_F(ResolverBehaviorTest, StmtForLoopEmpty_NoExit) {
auto* stmt = For(Source{{12, 34}}, nullptr, nullptr, nullptr, Block());
WrapInFunction(stmt);
ASSERT_TRUE(r()->Resolve()) << r()->error();
auto* sem = Sem().Get(stmt);
EXPECT_TRUE(sem->Behaviors().Empty());
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: for-loop does not exit");
}
TEST_F(ResolverBehaviorTest, StmtForLoopBreak) {
@@ -254,14 +253,13 @@ TEST_F(ResolverBehaviorTest, StmtForLoopBreak) {
EXPECT_EQ(sem->Behaviors(), sem::Behavior::kNext);
}
TEST_F(ResolverBehaviorTest, StmtForLoopContinue) {
auto* stmt = For(nullptr, nullptr, nullptr, Block(Continue()));
TEST_F(ResolverBehaviorTest, StmtForLoopContinue_NoExit) {
auto* stmt =
For(Source{{12, 34}}, nullptr, nullptr, nullptr, Block(Continue()));
WrapInFunction(stmt);
ASSERT_TRUE(r()->Resolve()) << r()->error();
auto* sem = Sem().Get(stmt);
EXPECT_TRUE(sem->Behaviors().Empty());
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: for-loop does not exit");
}
TEST_F(ResolverBehaviorTest, StmtForLoopDiscard) {
@@ -414,14 +412,12 @@ TEST_F(ResolverBehaviorTest, StmtLetDecl_RHSDiscardOrNext) {
sem::Behaviors(sem::Behavior::kDiscard, sem::Behavior::kNext));
}
TEST_F(ResolverBehaviorTest, StmtLoopEmpty) {
auto* stmt = Loop(Block());
TEST_F(ResolverBehaviorTest, StmtLoopEmpty_NoExit) {
auto* stmt = Loop(Source{{12, 34}}, Block());
WrapInFunction(stmt);
ASSERT_TRUE(r()->Resolve()) << r()->error();
auto* sem = Sem().Get(stmt);
EXPECT_TRUE(sem->Behaviors().Empty());
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: loop does not exit");
}
TEST_F(ResolverBehaviorTest, StmtLoopBreak) {
@@ -434,14 +430,12 @@ TEST_F(ResolverBehaviorTest, StmtLoopBreak) {
EXPECT_EQ(sem->Behaviors(), sem::Behavior::kNext);
}
TEST_F(ResolverBehaviorTest, StmtLoopContinue) {
auto* stmt = Loop(Block(Continue()));
TEST_F(ResolverBehaviorTest, StmtLoopContinue_NoExit) {
auto* stmt = Loop(Source{{12, 34}}, Block(Continue()));
WrapInFunction(stmt);
ASSERT_TRUE(r()->Resolve()) << r()->error();
auto* sem = Sem().Get(stmt);
EXPECT_TRUE(sem->Behaviors().Empty());
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: loop does not exit");
}
TEST_F(ResolverBehaviorTest, StmtLoopDiscard) {
@@ -464,14 +458,12 @@ TEST_F(ResolverBehaviorTest, StmtLoopReturn) {
EXPECT_EQ(sem->Behaviors(), sem::Behavior::kReturn);
}
TEST_F(ResolverBehaviorTest, StmtLoopEmpty_ContEmpty) {
auto* stmt = Loop(Block(), Block());
TEST_F(ResolverBehaviorTest, StmtLoopEmpty_ContEmpty_NoExit) {
auto* stmt = Loop(Source{{12, 34}}, Block(), Block());
WrapInFunction(stmt);
ASSERT_TRUE(r()->Resolve()) << r()->error();
auto* sem = Sem().Get(stmt);
EXPECT_TRUE(sem->Behaviors().Empty());
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: loop does not exit");
}
TEST_F(ResolverBehaviorTest, StmtLoopEmpty_ContBreak) {

View File

@@ -200,7 +200,7 @@ TEST_F(ResolverTest, Stmt_Loop) {
auto* body_lhs = Expr("v");
auto* body_rhs = Expr(2.3f);
auto* body = Block(Assign(body_lhs, body_rhs));
auto* body = Block(Assign(body_lhs, body_rhs), Break());
auto* continuing_lhs = Expr("v");
auto* continuing_rhs = Expr(2.3f);

View File

@@ -1426,7 +1426,19 @@ bool Resolver::ValidateElseStatement(const sem::ElseStatement* stmt) {
return true;
}
bool Resolver::ValidateLoopStatement(const sem::LoopStatement* stmt) {
if (stmt->Behaviors().Empty()) {
AddError("loop does not exit", stmt->Declaration()->source.Begin());
return false;
}
return true;
}
bool Resolver::ValidateForLoopStatement(const sem::ForLoopStatement* stmt) {
if (stmt->Behaviors().Empty()) {
AddError("for-loop does not exit", stmt->Declaration()->source.Begin());
return false;
}
if (auto* cond = stmt->Condition()) {
auto* cond_ty = cond->Type()->UnwrapRef();
if (!cond_ty->Is<sem::Bool>()) {

View File

@@ -481,6 +481,7 @@ note: identifier 'z' referenced in continuing block here)");
TEST_F(ResolverValidationTest,
Stmt_Loop_ContinueInLoopBodyAfterDecl_UsageInContinuing_InBlocks) {
// loop {
// if (false) { break; }
// var z : i32;
// {{{continue;}}}
// continue; // Ok
@@ -490,7 +491,8 @@ TEST_F(ResolverValidationTest,
// }
// }
auto* body = Block(Decl(Var("z", ty.i32(), ast::StorageClass::kNone)),
auto* body = Block(If(false, Block(Break())), //
Decl(Var("z", ty.i32(), ast::StorageClass::kNone)),
Block(Block(Block(Continue()))));
auto* continuing = Block(Assign(Expr("z"), 2));
auto* loop_stmt = Loop(body, continuing);
@@ -636,7 +638,7 @@ TEST_F(ResolverValidationTest,
// break;
// }
// var z : i32;
//
// break;
// continuing {
// z = 2;
// }
@@ -645,8 +647,9 @@ TEST_F(ResolverValidationTest,
auto* inner_loop = Loop(Block( //
If(true, Block(Continue())), //
Break()));
auto* body =
Block(inner_loop, Decl(Var("z", ty.i32(), ast::StorageClass::kNone)));
auto* body = Block(inner_loop, //
Decl(Var("z", ty.i32(), ast::StorageClass::kNone)), //
Break());
auto* continuing = Block(Assign("z", 2));
auto* loop_stmt = Loop(body, continuing);
WrapInFunction(loop_stmt);
@@ -662,7 +665,7 @@ TEST_F(ResolverValidationTest,
// break;
// }
// var z : i32;
//
// break;
// continuing {
// if (true) {
// z = 2;
@@ -672,8 +675,9 @@ TEST_F(ResolverValidationTest,
auto* inner_loop = Loop(Block(If(true, Block(Continue())), //
Break()));
auto* body =
Block(inner_loop, Decl(Var("z", ty.i32(), ast::StorageClass::kNone)));
auto* body = Block(inner_loop, //
Decl(Var("z", ty.i32(), ast::StorageClass::kNone)), //
Break());
auto* continuing = Block(If(Expr(true), Block(Assign("z", 2))));
auto* loop_stmt = Loop(body, continuing);
WrapInFunction(loop_stmt);
@@ -689,19 +693,22 @@ TEST_F(ResolverValidationTest,
// break;
// }
// var z : i32;
//
// break;
// continuing {
// loop {
// z = 2;
// break;
// }
// }
// }
auto* inner_loop = Loop(Block(If(true, Block(Continue())), //
Break()));
auto* body =
Block(inner_loop, Decl(Var("z", ty.i32(), ast::StorageClass::kNone)));
auto* continuing = Block(Loop(Block(Assign("z", 2))));
auto* body = Block(inner_loop, //
Decl(Var("z", ty.i32(), ast::StorageClass::kNone)), //
Break());
auto* continuing = Block(Loop(Block(Assign("z", 2), //
Break())));
auto* loop_stmt = Loop(body, continuing);
WrapInFunction(loop_stmt);
@@ -712,7 +719,7 @@ TEST_F(ResolverTest, Stmt_Loop_ContinueInLoopBodyAfterDecl_UsageInContinuing) {
// loop {
// var z : i32;
// if (true) { continue; }
//
// break;
// continuing {
// z = 2;
// }
@@ -720,7 +727,8 @@ TEST_F(ResolverTest, Stmt_Loop_ContinueInLoopBodyAfterDecl_UsageInContinuing) {
auto error_loc = Source{{12, 34}};
auto* body = Block(Decl(Var("z", ty.i32(), ast::StorageClass::kNone)),
If(true, Block(Continue())));
If(true, Block(Continue())), //
Break());
auto* continuing = Block(Assign(Expr(error_loc, "z"), 2));
auto* loop_stmt = Loop(body, continuing);
WrapInFunction(loop_stmt);
@@ -748,6 +756,7 @@ TEST_F(ResolverTest, Stmt_Loop_ReturnInContinuing_Direct) {
TEST_F(ResolverTest, Stmt_Loop_ReturnInContinuing_Indirect) {
// loop {
// if (false) { break; }
// continuing {
// loop {
// return;
@@ -755,11 +764,11 @@ TEST_F(ResolverTest, Stmt_Loop_ReturnInContinuing_Indirect) {
// }
// }
WrapInFunction(Loop( // outer loop
Block(), // outer loop block
Block(Source{{56, 78}}, // outer loop continuing block
Loop( // inner loop
Block( // inner loop block
WrapInFunction(Loop( // outer loop
Block(If(false, Block(Break()))), // 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());
@@ -789,16 +798,17 @@ TEST_F(ResolverTest, Stmt_Loop_DiscardInContinuing_Direct) {
TEST_F(ResolverTest, Stmt_Loop_DiscardInContinuing_Indirect) {
// loop {
// if (false) { break; }
// 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
WrapInFunction(Loop( // outer loop
Block(If(false, Block(Break()))), // outer loop block
Block(Source{{56, 78}}, // outer loop continuing block
Loop( // inner loop
Block( // inner loop block
Discard(Source{{12, 34}}))))));
EXPECT_FALSE(r()->Resolve());
@@ -854,19 +864,23 @@ TEST_F(ResolverTest, Stmt_Loop_ContinueInContinuing_Direct) {
TEST_F(ResolverTest, Stmt_Loop_ContinueInContinuing_Indirect) {
// loop {
// if (false) { break; }
// continuing {
// loop {
// if (false) { break; }
// continue;
// }
// }
// }
WrapInFunction(Loop( // outer loop
Block(), // outer loop block
Block( // outer loop continuing block
Loop( // inner loop
Block( // inner loop block
Continue(Source{{12, 34}}))))));
WrapInFunction(Loop( // outer loop
Block( // outer loop block
If(false, Block(Break()))), // if (false) { break; }
Block( // outer loop continuing block
Loop( // inner loop
Block( // inner loop block
If(false, Block(Break())), // if (false) { break; }
Continue(Source{{12, 34}})))))); // continue
EXPECT_TRUE(r()->Resolve()) << r()->error();
}
@@ -970,13 +984,14 @@ TEST_F(ResolverTest, Stmt_ForLoop_ContinueInContinuing_Direct) {
}
TEST_F(ResolverTest, Stmt_ForLoop_ContinueInContinuing_Indirect) {
// for(;; loop { continue }) {
// for(;; loop { if (false) { break; } continue }) {
// break;
// }
WrapInFunction(For(nullptr, nullptr,
Loop( //
Block(Continue(Source{{12, 34}}))), //
Block(If(false, Block(Break())), //
Continue(Source{{12, 34}}))), //
Block(Break())));
EXPECT_TRUE(r()->Resolve()) << r()->error();
@@ -1004,7 +1019,8 @@ TEST_F(ResolverTest, Stmt_ForLoop_CondIsNotBool) {
}
TEST_F(ResolverValidationTest, Stmt_ContinueInLoop) {
WrapInFunction(Loop(Block(Continue(Source{{12, 34}}))));
WrapInFunction(Loop(Block(If(false, Block(Break())), //
Continue(Source{{12, 34}}))));
EXPECT_TRUE(r()->Resolve()) << r()->error();
}
@@ -1015,20 +1031,21 @@ TEST_F(ResolverValidationTest, Stmt_ContinueNotInLoop) {
}
TEST_F(ResolverValidationTest, Stmt_BreakInLoop) {
WrapInFunction(Loop(Block(create<ast::BreakStatement>(Source{{12, 34}}))));
WrapInFunction(Loop(Block(Break(Source{{12, 34}}))));
EXPECT_TRUE(r()->Resolve()) << r()->error();
}
TEST_F(ResolverValidationTest, Stmt_BreakInSwitch) {
WrapInFunction(Loop(Block(Switch(
Expr(1),
Case(Expr(1), Block(create<ast::BreakStatement>(Source{{12, 34}}))),
DefaultCase()))));
WrapInFunction(Loop(Block(Switch(Expr(1), //
Case(Expr(1), //
Block(Break())), //
DefaultCase()), //
Break()))); //
EXPECT_TRUE(r()->Resolve()) << r()->error();
}
TEST_F(ResolverValidationTest, Stmt_BreakNotInLoopOrSwitch) {
WrapInFunction(create<ast::BreakStatement>(Source{{12, 34}}));
WrapInFunction(Break(Source{{12, 34}}));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 error: break statement must be in a loop or switch case");