encounter/dawn-cmake
1
0
Fork 0
mirror of https://github.com/encounter/dawn-cmake.git synced 2025-12-09 21:47:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

Remove if-break deprecation

Browse Source 
This CL removes support or if-break and requires the use of break-if.

Bug: tint:1724
Change-Id: I8311de2f0ce11b5af7fada71d258ae441f9e42f8
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/111100
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Dan Sinclair <dsinclair@chromium.org>
This commit is contained in:
dan sinclair
2022-11-23 02:14:05 +00:00
committed by Dawn LUCI CQ
parent 669e15e139
commit 0acbb4e047
69 changed files with 96 additions and 560 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
src/tint/resolver/resolver_behavior_test.cc
View File

@@ -557,16 +557,6 @@ TEST_F(ResolverBehaviorTest, StmtLoopEmpty_ContEmpty_NoExit) {
EXPECT_EQ(r()->error(), "12:34 error: loop does not exit");
}
TEST_F(ResolverBehaviorTest, StmtLoopEmpty_ContIfTrueBreak) {
auto* stmt = Loop(Block(), Block(If(true, Block(Break()))));
WrapInFunction(stmt);
ASSERT_TRUE(r()->Resolve()) << r()->error();
auto* sem = Sem().Get(stmt);
EXPECT_EQ(sem->Behaviors(), sem::Behavior::kNext);
}
TEST_F(ResolverBehaviorTest, StmtLoopEmpty_BreakIf) {
auto* stmt = Loop(Block(), Block(BreakIf(true)));
WrapInFunction(stmt);

78
src/tint/resolver/validation_test.cc
View File

@@ -1029,10 +1029,10 @@ TEST_F(ResolverValidationTest, Stmt_BreakInIfTrueInContinuing) {
// }
// }
WrapInFunction(Loop(Block(), cont));
EXPECT_TRUE(r()->Resolve()) << r()->error();
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 warning: use of deprecated language feature: `break` must not be used to exit "
"from a continuing block. Use `break-if` instead.");
"12:34 error: `break` must not be used to exit from a continuing block. "
"Use `break-if` instead.");
}
TEST_F(ResolverValidationTest, Stmt_BreakInIfElseInContinuing) {
@@ -1043,10 +1043,10 @@ TEST_F(ResolverValidationTest, Stmt_BreakInIfElseInContinuing) {
// }
// }
WrapInFunction(Loop(Block(), cont));
EXPECT_TRUE(r()->Resolve()) << r()->error();
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 warning: use of deprecated language feature: `break` must not be used to exit "
"from a continuing block. Use `break-if` instead.");
"12:34 error: `break` must not be used to exit from a continuing block. "
"Use `break-if` instead.");
}
TEST_F(ResolverValidationTest, Stmt_BreakInContinuing) {
@@ -1056,12 +1056,8 @@ TEST_F(ResolverValidationTest, Stmt_BreakInContinuing) {
WrapInFunction(Loop(Block(), cont));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 warning: use of deprecated language feature: `break` must not be used to exit "
"from a continuing block. Use `break-if` instead.\n"
"12:34 error: break statement in a continuing block must be the single "
"statement of an if statement's true or false block, and that if "
"statement must be the last statement of the continuing block\n"
"12:34 note: break statement is not directly in if statement block");
"12:34 error: `break` must not be used to exit from a continuing block. "
"Use `break-if` instead.");
}
TEST_F(ResolverValidationTest, Stmt_BreakInIfInIfInContinuing) {
@@ -1075,13 +1071,8 @@ TEST_F(ResolverValidationTest, Stmt_BreakInIfInIfInContinuing) {
WrapInFunction(Loop(Block(), cont));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 warning: use of deprecated language feature: `break` must not be used to exit "
"from a continuing block. Use `break-if` instead.\n"
"12:34 error: break statement in a continuing block must be the single "
"statement of an if statement's true or false block, and that if "
"statement must be the last statement of the continuing block\n"
"56:78 note: if statement containing break statement is not directly in "
"continuing block");
"12:34 error: `break` must not be used to exit from a continuing block. "
"Use `break-if` instead.");
}
TEST_F(ResolverValidationTest, Stmt_BreakInIfTrueMultipleStmtsInContinuing) {
@@ -1094,12 +1085,8 @@ TEST_F(ResolverValidationTest, Stmt_BreakInIfTrueMultipleStmtsInContinuing) {
WrapInFunction(Loop(Block(), cont));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 warning: use of deprecated language feature: `break` must not be used to exit "
"from a continuing block. Use `break-if` instead.\n"
"12:34 error: break statement in a continuing block must be the single "
"statement of an if statement's true or false block, and that if "
"statement must be the last statement of the continuing block\n"
"56:78 note: if statement block contains multiple statements");
"12:34 error: `break` must not be used to exit from a continuing block. "
"Use `break-if` instead.");
}
TEST_F(ResolverValidationTest, Stmt_BreakInIfElseMultipleStmtsInContinuing) {
@@ -1113,12 +1100,8 @@ TEST_F(ResolverValidationTest, Stmt_BreakInIfElseMultipleStmtsInContinuing) {
WrapInFunction(Loop(Block(), cont));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 warning: use of deprecated language feature: `break` must not be used to exit "
"from a continuing block. Use `break-if` instead.\n"
"12:34 error: break statement in a continuing block must be the single "
"statement of an if statement's true or false block, and that if "
"statement must be the last statement of the continuing block\n"
"56:78 note: if statement block contains multiple statements");
"12:34 error: `break` must not be used to exit from a continuing block. "
"Use `break-if` instead.");
}
TEST_F(ResolverValidationTest, Stmt_BreakInIfElseIfInContinuing) {
@@ -1131,12 +1114,8 @@ TEST_F(ResolverValidationTest, Stmt_BreakInIfElseIfInContinuing) {
WrapInFunction(Loop(Block(), cont));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 warning: use of deprecated language feature: `break` must not be used to exit "
"from a continuing block. Use `break-if` instead.\n"
"12:34 error: break statement in a continuing block must be the single "
"statement of an if statement's true or false block, and that if "
"statement must be the last statement of the continuing block\n"
"56:78 note: else has condition");
"12:34 error: `break` must not be used to exit from a continuing block. "
"Use `break-if` instead.");
}
TEST_F(ResolverValidationTest, Stmt_BreakInIfNonEmptyElseInContinuing) {
@@ -1150,12 +1129,8 @@ TEST_F(ResolverValidationTest, Stmt_BreakInIfNonEmptyElseInContinuing) {
WrapInFunction(Loop(Block(), cont));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 warning: use of deprecated language feature: `break` must not be used to exit "
"from a continuing block. Use `break-if` instead.\n"
"12:34 error: break statement in a continuing block must be the single "
"statement of an if statement's true or false block, and that if "
"statement must be the last statement of the continuing block\n"
"56:78 note: non-empty false block");
"12:34 error: `break` must not be used to exit from a continuing block. "
"Use `break-if` instead.");
}
TEST_F(ResolverValidationTest, Stmt_BreakInIfElseNonEmptyTrueInContinuing) {
@@ -1169,12 +1144,8 @@ TEST_F(ResolverValidationTest, Stmt_BreakInIfElseNonEmptyTrueInContinuing) {
WrapInFunction(Loop(Block(), cont));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 warning: use of deprecated language feature: `break` must not be used to exit "
"from a continuing block. Use `break-if` instead.\n"
"12:34 error: break statement in a continuing block must be the single "
"statement of an if statement's true or false block, and that if "
"statement must be the last statement of the continuing block\n"
"56:78 note: non-empty true block");
"12:34 error: `break` must not be used to exit from a continuing block. "
"Use `break-if` instead.");
}
TEST_F(ResolverValidationTest, Stmt_BreakInIfInContinuingNotLast) {
@@ -1187,13 +1158,8 @@ TEST_F(ResolverValidationTest, Stmt_BreakInIfInContinuingNotLast) {
WrapInFunction(Loop(Block(), cont));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 warning: use of deprecated language feature: `break` must not be used to exit "
"from a continuing block. Use `break-if` instead.\n"
"12:34 error: break statement in a continuing block must be the single "
"statement of an if statement's true or false block, and that if "
"statement must be the last statement of the continuing block\n"
"56:78 note: if statement containing break statement is not the last "
"statement of the continuing block");
"12:34 error: `break` must not be used to exit from a continuing block. "
"Use `break-if` instead.");
}
TEST_F(ResolverValidationTest, Stmt_BreakNotInLoopOrSwitch) {

61
src/tint/resolver/validator.cc
View File

@@ -1438,64 +1438,11 @@ bool Validator::BreakStatement(const sem::Statement* stmt,
AddError("break statement must be in a loop or switch case", stmt->Declaration()->source);
return false;
}
if (auto* continuing = ClosestContinuing(/*stop_at_loop*/ true, current_statement)) {
AddWarning(
"use of deprecated language feature: `break` must not be used to exit from "
"a continuing block. Use `break-if` instead.",
if (ClosestContinuing(/*stop_at_loop*/ true, current_statement) != nullptr) {
AddError(
"`break` must not be used to exit from a continuing block. Use `break-if` instead.",
stmt->Declaration()->source);
auto fail = [&](const char* note_msg, const Source& note_src) {
constexpr const char* kErrorMsg =
"break statement in a continuing block must be the single statement of an if "
"statement's true or false block, and that if statement must be the last statement "
"of the continuing block";
AddError(kErrorMsg, stmt->Declaration()->source);
AddNote(note_msg, note_src);
return false;
};
if (auto* block = stmt->Parent()->As<sem::BlockStatement>()) {
auto* block_parent = block->Parent();
auto* if_stmt = block_parent->As<sem::IfStatement>();
if (!if_stmt) {
return fail("break statement is not directly in if statement block",
stmt->Declaration()->source);
}
if (block->Declaration()->statements.Length() != 1) {
return fail("if statement block contains multiple statements",
block->Declaration()->source);
}
if (if_stmt->Parent()->Is<sem::IfStatement>()) {
return fail("else has condition", if_stmt->Declaration()->source);
}
bool el_contains_break = block->Declaration() == if_stmt->Declaration()->else_statement;
if (el_contains_break) {
if (auto* true_block = if_stmt->Declaration()->body; !true_block->Empty()) {
return fail("non-empty true block", true_block->source);
}
} else {
auto* else_stmt = if_stmt->Declaration()->else_statement;
if (else_stmt) {
return fail("non-empty false block", else_stmt->source);
}
}
if (if_stmt->Parent()->Declaration() != continuing) {
return fail(
"if statement containing break statement is not directly in continuing block",
if_stmt->Declaration()->source);
}
if (auto* cont_block = continuing->As<ast::BlockStatement>()) {
if (if_stmt->Declaration() != cont_block->Last()) {
return fail(
"if statement containing break statement is not the last statement of the "
"continuing block",
if_stmt->Declaration()->source);
}
}
}
return false;
}
return true;
}

4
src/tint/transform/remove_unreachable_statements_test.cc
View File

@@ -209,9 +209,7 @@ fn f() {
loop {
continuing {
if (true) {
break;
}
break if true;
}
}
var preserve_me = 1;

48
src/tint/writer/spirv/builder.cc
View File

@@ -3403,54 +3403,6 @@ bool Builder::GenerateConditionalBlock(const ast::Expression* cond,
}
bool Builder::GenerateIfStatement(const ast::IfStatement* stmt) {
if (!continuing_stack_.empty() &&
stmt == continuing_stack_.back().last_statement->As<ast::IfStatement>()) {
const ContinuingInfo& ci = continuing_stack_.back();
// Match one of two patterns: the break-if and break-unless patterns.
//
// The break-if pattern:
// continuing { ...
// if (cond) { break; }
// }
//
// The break-unless pattern:
// continuing { ...
// if (cond) {} else {break;}
// }
//
// TODO(crbug.com/tint/1451): Remove this when the if break construct is made an error.
auto is_just_a_break = [](const ast::BlockStatement* block) {
return block && (block->statements.Length() == 1) &&
block->Last()->Is<ast::BreakStatement>();
};
if (is_just_a_break(stmt->body) && stmt->else_statement == nullptr) {
// It's a break-if.
TINT_ASSERT(Writer, !backedge_stack_.empty());
const auto cond_id = GenerateExpressionWithLoadIfNeeded(stmt->condition);
if (!cond_id) {
return false;
}
backedge_stack_.back() = Backedge(
spv::Op::OpBranchConditional,
{Operand(cond_id), Operand(ci.break_target_id), Operand(ci.loop_header_id)});
return true;
} else if (stmt->body->Empty()) {
auto* es_block = As<ast::BlockStatement>(stmt->else_statement);
if (es_block && is_just_a_break(es_block)) {
// It's a break-unless.
TINT_ASSERT(Writer, !backedge_stack_.empty());
const auto cond_id = GenerateExpressionWithLoadIfNeeded(stmt->condition);
if (!cond_id) {
return false;
}
backedge_stack_.back() = Backedge(
spv::Op::OpBranchConditional,
{Operand(cond_id), Operand(ci.loop_header_id), Operand(ci.break_target_id)});
return true;
}
}
}
if (!GenerateConditionalBlock(stmt->condition, stmt->body, stmt->else_statement)) {
return false;
}

91
src/tint/writer/spirv/builder_loop_test.cc
View File

@@ -332,44 +332,6 @@ OpBranchConditional %10 %2 %1
)");
}
TEST_F(BuilderTest, Loop_WithContinuing_BreakUnless_ConditionIsVar) {
// loop {
// continuing {
// var cond = true;
// if (cond) {} else { break; }
// }
// }
auto* cond_var = Decl(Var("cond", Expr(true)));
auto* if_stmt = If(Expr("cond"), Block(), Else(Block(Break())));
auto* continuing = Block(cond_var, if_stmt);
auto* loop = Loop(Block(), continuing);
WrapInFunction(loop);
spirv::Builder& b = Build();
b.push_function(Function{});
EXPECT_TRUE(b.GenerateLoopStatement(loop)) << b.error();
EXPECT_EQ(DumpInstructions(b.types()), R"(%5 = OpTypeBool
%6 = OpConstantTrue %5
%8 = OpTypePointer Function %5
%9 = OpConstantNull %5
)");
EXPECT_EQ(DumpInstructions(b.functions()[0].instructions()),
R"(OpBranch %1
%1 = OpLabel
OpLoopMerge %2 %3 None
OpBranch %4
%4 = OpLabel
OpBranch %3
%3 = OpLabel
OpStore %7 %6
%10 = OpLoad %5 %7
OpBranchConditional %10 %1 %2
%2 = OpLabel
)");
}
TEST_F(BuilderTest, Loop_WithContinuing_BreakIf_Nested) {
// Make sure the right backedge and break target are used.
// loop {
@@ -421,58 +383,5 @@ OpBranchConditional %10 %2 %1
)");
}
TEST_F(BuilderTest, Loop_WithContinuing_BreakUnless_Nested) {
// Make sure the right backedge and break target are used.
// loop {
// continuing {
// loop {
// continuing {
// if (true) {} else { break; }
// }
// }
// if (true) {} else { break; }
// }
// }
auto* inner_if_stmt = If(Expr(true), Block(), Else(Block(Break())));
auto* inner_continuing = Block(inner_if_stmt);
auto* inner_loop = Loop(Block(), inner_continuing);
auto* outer_if_stmt = If(Expr(true), Block(), Else(Block(Break())));
auto* outer_continuing = Block(inner_loop, outer_if_stmt);
auto* outer_loop = Loop(Block(), outer_continuing);
WrapInFunction(outer_loop);
spirv::Builder& b = Build();
b.push_function(Function{});
EXPECT_TRUE(b.GenerateLoopStatement(outer_loop)) << b.error();
EXPECT_EQ(DumpInstructions(b.types()), R"(%9 = OpTypeBool
%10 = OpConstantTrue %9
)");
EXPECT_EQ(DumpInstructions(b.functions()[0].instructions()),
R"(OpBranch %1
%1 = OpLabel
OpLoopMerge %2 %3 None
OpBranch %4
%4 = OpLabel
OpBranch %3
%3 = OpLabel
OpBranch %5
%5 = OpLabel
OpLoopMerge %6 %7 None
OpBranch %8
%8 = OpLabel
OpBranch %7
%7 = OpLabel
OpBranchConditional %10 %5 %6
%6 = OpLabel
OpBranchConditional %10 %1 %2
%2 = OpLabel
)");
}
} // namespace
} // namespace tint::writer::spirv