diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index eebee238aa..2f1adb3364 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -467,6 +467,7 @@ if(${TINT_BUILD_TESTS}) program_test.cc resolver/assignment_validation_test.cc resolver/builtins_validation_test.cc + resolver/control_block_validation_test.cc resolver/decoration_validation_test.cc resolver/function_validation_test.cc resolver/host_shareable_validation_test.cc @@ -510,7 +511,6 @@ if(${TINT_BUILD_TESTS}) utils/tmpfile_test.cc utils/tmpfile.h utils/unique_vector_test.cc - validator/validator_control_block_test.cc validator/validator_decoration_test.cc validator/validator_function_test.cc validator/validator_test.cc diff --git a/src/ast/module_clone_test.cc b/src/ast/module_clone_test.cc index 87710e7b0b..4decb9297a 100644 --- a/src/ast/module_clone_test.cc +++ b/src/ast/module_clone_test.cc @@ -84,10 +84,10 @@ fn f1(p0 : f32, p1 : i32) -> f32 { } } switch(l2) { - case 0: { + case 0u: { break; } - case 1: { + case 1u: { return f0(true); } default: { diff --git a/src/program_builder.h b/src/program_builder.h index ffe747623a..12b9d41af7 100644 --- a/src/program_builder.h +++ b/src/program_builder.h @@ -23,6 +23,7 @@ #include "src/ast/binary_expression.h" #include "src/ast/bool_literal.h" #include "src/ast/call_expression.h" +#include "src/ast/case_statement.h" #include "src/ast/float_literal.h" #include "src/ast/if_statement.h" #include "src/ast/loop_statement.h" @@ -35,6 +36,7 @@ #include "src/ast/struct_member_align_decoration.h" #include "src/ast/struct_member_offset_decoration.h" #include "src/ast/struct_member_size_decoration.h" +#include "src/ast/switch_statement.h" #include "src/ast/type_constructor_expression.h" #include "src/ast/uint_literal.h" #include "src/ast/variable_decl_statement.h" @@ -1150,11 +1152,15 @@ class ProgramBuilder { } /// Creates a ast::AssignmentStatement with input lhs and rhs expressions - /// @param lhs the left hand side expression - /// @param rhs the right hand side expression + /// @param lhs the left hand side expression initializer + /// @param rhs the right hand side expression initializer /// @returns the assignment statement pointer - ast::AssignmentStatement* Assign(ast::Expression* lhs, ast::Expression* rhs) { - return create(lhs, rhs); + template + ast::AssignmentStatement* Assign(LhsExpressionInit&& lhs, + RhsExpressionInit&& rhs) { + return create( + Expr(std::forward(lhs)), + Expr(std::forward(rhs))); } /// Creates a ast::LoopStatement with input body and optional continuing @@ -1173,6 +1179,43 @@ class ProgramBuilder { return create(var); } + /// Creates a ast::SwitchStatement with input expression and cases + /// @param condition the condition expression initializer + /// @param cases case statements + /// @returns the switch statement pointer + template + ast::SwitchStatement* Switch(ExpressionInit&& condition, Cases&&... cases) { + return create( + Expr(std::forward(condition)), + ast::CaseStatementList{std::forward(cases)...}); + } + + /// Creates a ast::CaseStatement with input list of selectors, and body + /// @param selectors list of selectors + /// @param body the case body + /// @returns the case statement pointer + ast::CaseStatement* Case(ast::CaseSelectorList selectors, + ast::BlockStatement* body = nullptr) { + return create(std::move(selectors), + body ? body : Block()); + } + + /// Convenient overload that takes a single selector + /// @param selector a single case selector + /// @param body the case body + /// @returns the case statement pointer + ast::CaseStatement* Case(ast::IntLiteral* selector, + ast::BlockStatement* body = nullptr) { + return Case(ast::CaseSelectorList{selector}, body); + } + + /// Convenience function that creates a 'default' ast::CaseStatement + /// @param body the case body + /// @returns the case statement pointer + ast::CaseStatement* DefaultCase(ast::BlockStatement* body = nullptr) { + return Case(ast::CaseSelectorList{}, body); + } + /// Sets the current builder source to `src` /// @param src the Source used for future create() calls void SetSource(const Source& src) { diff --git a/src/validator/validator_control_block_test.cc b/src/resolver/control_block_validation_test.cc similarity index 76% rename from src/validator/validator_control_block_test.cc rename to src/resolver/control_block_validation_test.cc index fcca90c85c..4e17660e5d 100644 --- a/src/validator/validator_control_block_test.cc +++ b/src/resolver/control_block_validation_test.cc @@ -1,4 +1,4 @@ -// Copyright 2020 The Tint Authors. +// Copyright 2021 The Tint Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -13,15 +13,17 @@ // limitations under the License. #include "src/ast/fallthrough_statement.h" -#include "src/validator/validator_test_helper.h" +#include "src/ast/switch_statement.h" +#include "src/resolver/resolver_test_helper.h" namespace tint { namespace { -class ValidateControlBlockTest : public ValidatorTestHelper, - public testing::Test {}; +class ResolverControlBlockValidationTest : public resolver::TestHelper, + public testing::Test {}; -TEST_F(ValidateControlBlockTest, SwitchSelectorExpressionNoneIntegerType_Fail) { +TEST_F(ResolverControlBlockValidationTest, + SwitchSelectorExpressionNoneIntegerType_Fail) { // var a : f32 = 3.14; // switch (a) { // default: {} @@ -41,15 +43,13 @@ TEST_F(ValidateControlBlockTest, SwitchSelectorExpressionNoneIntegerType_Fail) { WrapInFunction(block); - ValidatorImpl& v = Build(); - - EXPECT_FALSE(v.ValidateStatements(block)); - EXPECT_EQ(v.error(), - "12:34 v-0025: switch statement selector expression must be " + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error v-0025: switch statement selector expression must be " "of a scalar integer type"); } -TEST_F(ValidateControlBlockTest, SwitchWithoutDefault_Fail) { +TEST_F(ResolverControlBlockValidationTest, SwitchWithoutDefault_Fail) { // var a : i32 = 2; // switch (a) { // case 1: {} @@ -71,15 +71,12 @@ TEST_F(ValidateControlBlockTest, SwitchWithoutDefault_Fail) { WrapInFunction(block); - ValidatorImpl& v = Build(); - - EXPECT_FALSE(v.ValidateStatements(block)); - EXPECT_EQ(v.error(), - "12:34 v-0008: switch statement must have exactly one default " - "clause"); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error: switch statement must have a default clause"); } -TEST_F(ValidateControlBlockTest, SwitchWithTwoDefault_Fail) { +TEST_F(ResolverControlBlockValidationTest, SwitchWithTwoDefault_Fail) { // var a : i32 = 2; // switch (a) { // default: {} @@ -101,28 +98,26 @@ TEST_F(ValidateControlBlockTest, SwitchWithTwoDefault_Fail) { ast::CaseSelectorList default_csl_2; auto* block_default_2 = create(ast::StatementList{}); - switch_body.push_back( - create(default_csl_2, block_default_2)); + switch_body.push_back(create( + Source{Source::Location{12, 34}}, default_csl_2, block_default_2)); auto* block = create(ast::StatementList{ create(var), - create(Source{Source::Location{12, 34}}, Expr("a"), - switch_body), + create(Expr("a"), switch_body), }); WrapInFunction(block); - ValidatorImpl& v = Build(); - - EXPECT_FALSE(v.ValidateStatements(block)); - EXPECT_EQ(v.error(), - "12:34 v-0008: switch statement must have exactly one default " - "clause"); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ( + r()->error(), + "12:34 error v-0008: switch statement must have exactly one default " + "clause"); } -TEST_F(ValidateControlBlockTest, +TEST_F(ResolverControlBlockValidationTest, SwitchConditionTypeMustMatchSelectorType2_Fail) { - // var a : i32 = 2; + // var a : u32 = 2; // switch (a) { // case 1: {} // default: {} @@ -146,15 +141,13 @@ TEST_F(ValidateControlBlockTest, }); WrapInFunction(block); - ValidatorImpl& v = Build(); - - EXPECT_FALSE(v.ValidateStatements(block)); - EXPECT_EQ(v.error(), - "12:34 v-0026: the case selector values must have the same " + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error v-0026: the case selector values must have the same " "type as the selector expression."); } -TEST_F(ValidateControlBlockTest, +TEST_F(ResolverControlBlockValidationTest, SwitchConditionTypeMustMatchSelectorType_Fail) { // var a : u32 = 2; // switch (a) { @@ -180,15 +173,14 @@ TEST_F(ValidateControlBlockTest, }); WrapInFunction(block); - ValidatorImpl& v = Build(); - - EXPECT_FALSE(v.ValidateStatements(block)); - EXPECT_EQ(v.error(), - "12:34 v-0026: the case selector values must have the same " + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + "12:34 error v-0026: the case selector values must have the same " "type as the selector expression."); } -TEST_F(ValidateControlBlockTest, NonUniqueCaseSelectorValueUint_Fail) { +TEST_F(ResolverControlBlockValidationTest, + NonUniqueCaseSelectorValueUint_Fail) { // var a : u32 = 3; // switch (a) { // case 0: {} @@ -220,15 +212,15 @@ TEST_F(ValidateControlBlockTest, NonUniqueCaseSelectorValueUint_Fail) { }); WrapInFunction(block); - ValidatorImpl& v = Build(); - - EXPECT_FALSE(v.ValidateStatements(block)); - EXPECT_EQ(v.error(), - "12:34 v-0027: a literal value must not appear more than once " - "in the case selectors for a switch statement: '2'"); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ( + r()->error(), + "12:34 error v-0027: a literal value must not appear more than once " + "in the case selectors for a switch statement: '2'"); } -TEST_F(ValidateControlBlockTest, NonUniqueCaseSelectorValueSint_Fail) { +TEST_F(ResolverControlBlockValidationTest, + NonUniqueCaseSelectorValueSint_Fail) { // var a : i32 = 2; // switch (a) { // case 10: {} @@ -262,15 +254,15 @@ TEST_F(ValidateControlBlockTest, NonUniqueCaseSelectorValueSint_Fail) { }); WrapInFunction(block); - ValidatorImpl& v = Build(); - - EXPECT_FALSE(v.ValidateStatements(block)); - EXPECT_EQ(v.error(), - "12:34 v-0027: a literal value must not appear more than once in " - "the case selectors for a switch statement: '10'"); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ( + r()->error(), + "12:34 error v-0027: a literal value must not appear more than once in " + "the case selectors for a switch statement: '10'"); } -TEST_F(ValidateControlBlockTest, LastClauseLastStatementIsFallthrough_Fail) { +TEST_F(ResolverControlBlockValidationTest, + LastClauseLastStatementIsFallthrough_Fail) { // var a : i32 = 2; // switch (a) { // default: { fallthrough; } @@ -292,15 +284,14 @@ TEST_F(ValidateControlBlockTest, LastClauseLastStatementIsFallthrough_Fail) { }); WrapInFunction(block); - ValidatorImpl& v = Build(); - - EXPECT_FALSE(v.ValidateStatements(block)); - EXPECT_EQ(v.error(), - "12:34 v-0028: a fallthrough statement must not appear as the " - "last statement in last clause of a switch"); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ( + r()->error(), + "12:34 error v-0028: a fallthrough statement must not appear as the " + "last statement in last clause of a switch"); } -TEST_F(ValidateControlBlockTest, SwitchCase_Pass) { +TEST_F(ResolverControlBlockValidationTest, SwitchCase_Pass) { // var a : i32 = 2; // switch (a) { // default: {} @@ -324,12 +315,10 @@ TEST_F(ValidateControlBlockTest, SwitchCase_Pass) { }); WrapInFunction(block); - ValidatorImpl& v = Build(); - - EXPECT_TRUE(v.ValidateStatements(block)) << v.error(); + EXPECT_TRUE(r()->Resolve()) << r()->error(); } -TEST_F(ValidateControlBlockTest, SwitchCaseAlias_Pass) { +TEST_F(ResolverControlBlockValidationTest, SwitchCaseAlias_Pass) { // type MyInt = u32; // var v: MyInt; // switch(v){ @@ -353,9 +342,7 @@ TEST_F(ValidateControlBlockTest, SwitchCaseAlias_Pass) { WrapInFunction(block); - ValidatorImpl& v = Build(); - - EXPECT_TRUE(v.ValidateStatements(block)) << v.error(); + EXPECT_TRUE(r()->Resolve()) << r()->error(); } } // namespace diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc index b4915cfa0a..189202ae28 100644 --- a/src/resolver/resolver.cc +++ b/src/resolver/resolver.cc @@ -409,15 +409,7 @@ bool Resolver::Statement(ast::Statement* stmt) { return Return(r); } if (auto* s = stmt->As()) { - if (!Expression(s->condition())) { - return false; - } - for (auto* case_stmt : s->body()) { - if (!CaseStatement(case_stmt)) { - return false; - } - } - return true; + return Switch(s); } if (auto* v = stmt->As()) { return VariableDeclStatement(v); @@ -1676,6 +1668,91 @@ bool Resolver::Return(ast::ReturnStatement* ret) { return result && ValidateReturn(ret); } +bool Resolver::ValidateSwitch(const ast::SwitchStatement* s) { + auto* cond_type = TypeOf(s->condition())->UnwrapAll(); + if (!cond_type->is_integer_scalar()) { + diagnostics_.add_error("v-0025", + "switch statement selector expression must be of a " + "scalar integer type", + s->condition()->source()); + return false; + } + + bool has_default = false; + std::unordered_set selector_set; + + for (auto* case_stmt : s->body()) { + if (case_stmt->IsDefault()) { + if (has_default) { + // More than one default clause + diagnostics_.add_error( + "v-0008", "switch statement must have exactly one default clause", + case_stmt->source()); + return false; + } + has_default = true; + } + + for (auto* selector : case_stmt->selectors()) { + if (cond_type != selector->type()) { + diagnostics_.add_error("v-0026", + "the case selector values must have the same " + "type as the selector expression.", + case_stmt->source()); + return false; + } + + auto v = selector->value_as_u32(); + if (selector_set.find(v) != selector_set.end()) { + diagnostics_.add_error( + "v-0027", + "a literal value must not appear more than once in " + "the case selectors for a switch statement: '" + + builder_->str(selector) + "'", + case_stmt->source()); + return false; + } + selector_set.emplace(v); + } + } + + if (!has_default) { + // No default clause + diagnostics_.add_error("switch statement must have a default clause", + s->source()); + return false; + } + + if (!s->body().empty()) { + auto* last_clause = s->body().back()->As(); + auto* last_stmt = last_clause->body()->last(); + if (last_stmt && last_stmt->Is()) { + diagnostics_.add_error("v-0028", + "a fallthrough statement must not appear as " + "the last statement in last clause of a switch", + last_stmt->source()); + return false; + } + } + + return true; +} + +bool Resolver::Switch(ast::SwitchStatement* s) { + if (!Expression(s->condition())) { + return false; + } + for (auto* case_stmt : s->body()) { + if (!CaseStatement(case_stmt)) { + return false; + } + } + if (!ValidateSwitch(s)) { + return false; + } + return true; +} + bool Resolver::ApplyStorageClassUsageToType(ast::StorageClass sc, type::Type* ty, Source usage) { diff --git a/src/resolver/resolver.h b/src/resolver/resolver.h index 13a0453f3c..3109dc6f2f 100644 --- a/src/resolver/resolver.h +++ b/src/resolver/resolver.h @@ -40,6 +40,7 @@ class Function; class IdentifierExpression; class MemberAccessorExpression; class ReturnStatement; +class SwitchStatement; class UnaryOpExpression; class Variable; } // namespace ast @@ -220,6 +221,7 @@ class Resolver { bool UnaryOp(ast::UnaryOpExpression*); bool VariableDeclStatement(const ast::VariableDeclStatement*); bool Return(ast::ReturnStatement* ret); + bool Switch(ast::SwitchStatement* s); // AST and Type validation methods // Each return true on success, false on failure. @@ -228,6 +230,7 @@ class Resolver { bool ValidateFunction(const ast::Function* func); bool ValidateStructure(const type::Struct* st); bool ValidateReturn(const ast::ReturnStatement* ret); + bool ValidateSwitch(const ast::SwitchStatement* s); /// @returns the semantic information for the array `arr`, building it if it /// hasn't been constructed already. If an error is raised, nullptr is diff --git a/src/resolver/resolver_test.cc b/src/resolver/resolver_test.cc index 53e35b0721..ac59bff017 100644 --- a/src/resolver/resolver_test.cc +++ b/src/resolver/resolver_test.cc @@ -248,14 +248,8 @@ TEST_F(ResolverTest, Stmt_Switch) { auto* lhs = Expr("v"); auto* rhs = Expr(2.3f); - auto* body = Block(create(lhs, rhs)); - ast::CaseSelectorList lit; - lit.push_back(create(ty.i32(), 3)); - - ast::CaseStatementList cases; - cases.push_back(create(lit, body)); - - auto* stmt = create(Expr(2), cases); + auto* stmt = + Switch(Expr(2), Case(Literal(3), Block(Assign(lhs, rhs))), DefaultCase()); WrapInFunction(v, stmt); EXPECT_TRUE(r()->Resolve()) << r()->error(); diff --git a/src/resolver/validation_test.cc b/src/resolver/validation_test.cc index 8536a22a38..11266c4d4a 100644 --- a/src/resolver/validation_test.cc +++ b/src/resolver/validation_test.cc @@ -666,12 +666,10 @@ TEST_F(ResolverValidationTest, Stmt_BreakInLoop) { } TEST_F(ResolverValidationTest, Stmt_BreakInSwitch) { - WrapInFunction(Loop(Block(create( - Expr(1), ast::CaseStatementList{ - create( - ast::CaseSelectorList{Literal(1)}, - Block(create(Source{{12, 34}}))), - })))); + WrapInFunction(Loop(Block(Switch( + Expr(1), + Case(Literal(1), Block(create(Source{{12, 34}}))), + DefaultCase())))); EXPECT_TRUE(r()->Resolve()) << r()->error(); } diff --git a/src/validator/validator_impl.cc b/src/validator/validator_impl.cc index 16187d9db1..f9aea11507 100644 --- a/src/validator/validator_impl.cc +++ b/src/validator/validator_impl.cc @@ -253,63 +253,13 @@ bool ValidatorImpl::ValidateStatement(const ast::Statement* stmt) { } bool ValidatorImpl::ValidateSwitch(const ast::SwitchStatement* s) { - auto* cond_type = program_->Sem().Get(s->condition())->Type()->UnwrapAll(); - if (!cond_type->is_integer_scalar()) { - add_error(s->condition()->source(), "v-0025", - "switch statement selector expression must be of a " - "scalar integer type"); - return false; - } - - int default_counter = 0; - std::unordered_set selector_set; + // TODO(amaiorano): Switch validation has moved to Resolver, but we need this + // logic to validate case statements for now. Remove once ValidateStatement() + // can be removed. for (auto* case_stmt : s->body()) { if (!ValidateStatement(case_stmt)) { return false; } - - if (case_stmt->IsDefault()) { - default_counter++; - } - - for (auto* selector : case_stmt->selectors()) { - if (cond_type != selector->type()) { - add_error(case_stmt->source(), "v-0026", - "the case selector values must have the same " - "type as the selector expression."); - return false; - } - - auto v = - static_cast(selector->type()->Is() - ? selector->As()->value() - : selector->As()->value()); - if (selector_set.count(v)) { - add_error(case_stmt->source(), "v-0027", - "a literal value must not appear more than once in " - "the case selectors for a switch statement: '" + - program_->str(selector) + "'"); - return false; - } - selector_set.emplace(v); - } - } - - if (default_counter != 1) { - add_error(s->source(), "v-0008", - "switch statement must have exactly one default clause"); - return false; - } - - auto* last_clause = s->body().back(); - auto* last_stmt_of_last_clause = - last_clause->As()->body()->last(); - if (last_stmt_of_last_clause && - last_stmt_of_last_clause->Is()) { - add_error(last_stmt_of_last_clause->source(), "v-0028", - "a fallthrough statement must not appear as " - "the last statement in last clause of a switch"); - return false; } return true; } diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc index 77401a8a09..bec06069a4 100644 --- a/src/writer/spirv/builder.cc +++ b/src/writer/spirv/builder.cc @@ -2726,7 +2726,8 @@ bool Builder::GenerateSwitchStatement(ast::SwitchStatement* stmt) { if (LastIsFallthrough(item->body())) { if (i == (body.size() - 1)) { - error_ = "fallthrough of last case statement is disallowed"; + // This case is caught by Resolver validation + TINT_UNREACHABLE(builder_.Diagnostics()); return false; } if (!push_function_inst(spv::Op::OpBranch, diff --git a/src/writer/spirv/builder_switch_test.cc b/src/writer/spirv/builder_switch_test.cc index f9d2d977e5..45077adc25 100644 --- a/src/writer/spirv/builder_switch_test.cc +++ b/src/writer/spirv/builder_switch_test.cc @@ -25,10 +25,10 @@ using BuilderTest = TestHelper; TEST_F(BuilderTest, Switch_Empty) { // switch (1) { + // default: {} // } - auto* expr = create(Expr(1), ast::CaseStatementList{}); - + auto* expr = Switch(1, DefaultCase()); WrapInFunction(expr); spirv::Builder& b = Build(); @@ -54,29 +54,15 @@ TEST_F(BuilderTest, Switch_WithCase) { // v = 1; // case 2: // v = 2; + // default: {} // } auto* v = Global("v", ty.i32(), ast::StorageClass::kPrivate); auto* a = Global("a", ty.i32(), ast::StorageClass::kPrivate); - auto* case_1_body = create( - ast::StatementList{create(Expr("v"), Expr(1))}); - - auto* case_2_body = create( - ast::StatementList{create(Expr("v"), Expr(2))}); - - ast::CaseSelectorList selector_1; - selector_1.push_back(Literal(1)); - - ast::CaseSelectorList selector_2; - selector_2.push_back(Literal(2)); - - ast::CaseStatementList cases; - cases.push_back(create(selector_1, case_1_body)); - cases.push_back(create(selector_2, case_2_body)); - - auto* expr = create(Expr("a"), cases); - + auto* expr = Switch("a", /**/ + Case(Literal(1), Block(Assign("v", 1))), + Case(Literal(2), Block(Assign("v", 2))), DefaultCase()); WrapInFunction(expr); auto* func = Func("a_func", {}, ty.void_(), ast::StatementList{}, @@ -127,28 +113,14 @@ TEST_F(BuilderTest, Switch_WithCase_Unsigned) { // v = 1; // case 2u: // v = 2; + // default: {} // } auto* v = Global("v", ty.i32(), ast::StorageClass::kPrivate); auto* a = Global("a", ty.u32(), ast::StorageClass::kPrivate); - auto* case_1_body = create( - ast::StatementList{create(Expr("v"), Expr(1))}); - - auto* case_2_body = create( - ast::StatementList{create(Expr("v"), Expr(2))}); - - ast::CaseSelectorList selector_1; - selector_1.push_back(Literal(1u)); - - ast::CaseSelectorList selector_2; - selector_2.push_back(Literal(2u)); - - ast::CaseStatementList cases; - cases.push_back(create(selector_1, case_1_body)); - cases.push_back(create(selector_2, case_2_body)); - - auto* expr = create(Expr("a"), cases); + auto* expr = Switch("a", Case(Literal(1u), Block(Assign("v", 1))), + Case(Literal(2u), Block(Assign("v", 2))), DefaultCase()); WrapInFunction(expr); @@ -199,7 +171,7 @@ OpFunctionEnd TEST_F(BuilderTest, Switch_WithDefault) { // switch(true) { - // default: + // default: {} // v = 1; // } @@ -259,7 +231,7 @@ TEST_F(BuilderTest, Switch_WithCaseAndDefault) { // v = 1; // case 2, 3: // v = 2; - // default: + // default: {} // v = 3; // } @@ -343,7 +315,7 @@ TEST_F(BuilderTest, Switch_CaseWithFallthrough) { // fallthrough; // case 2: // v = 2; - // default: + // default: {} // v = 3; // } @@ -420,43 +392,6 @@ OpFunctionEnd )"); } -TEST_F(BuilderTest, Switch_CaseFallthroughLastStatement) { - // switch(a) { - // case 1: - // v = 1; - // fallthrough; - // } - - auto* v = Global("v", ty.i32(), ast::StorageClass::kPrivate); - auto* a = Global("a", ty.i32(), ast::StorageClass::kPrivate); - - auto* case_1_body = create( - ast::StatementList{create(Expr("v"), Expr(1)), - create()}); - - ast::CaseSelectorList selector_1; - selector_1.push_back(Literal(1)); - - ast::CaseStatementList cases; - cases.push_back(create(selector_1, case_1_body)); - - auto* expr = create(Expr("a"), cases); - - WrapInFunction(expr); - - auto* func = Func("a_func", {}, ty.void_(), ast::StatementList{}, - ast::DecorationList{}); - - spirv::Builder& b = Build(); - - ASSERT_TRUE(b.GenerateGlobalVariable(v)) << b.error(); - ASSERT_TRUE(b.GenerateGlobalVariable(a)) << b.error(); - ASSERT_TRUE(b.GenerateFunction(func)) << b.error(); - - EXPECT_FALSE(b.GenerateSwitchStatement(expr)) << b.error(); - EXPECT_EQ(b.error(), "fallthrough of last case statement is disallowed"); -} - TEST_F(BuilderTest, Switch_WithNestedBreak) { // switch (a) { // case 1: @@ -464,26 +399,19 @@ TEST_F(BuilderTest, Switch_WithNestedBreak) { // break; // } // v = 1; - // } + // default: {} + // } auto* v = Global("v", ty.i32(), ast::StorageClass::kPrivate); auto* a = Global("a", ty.i32(), ast::StorageClass::kPrivate); - auto* if_body = create(ast::StatementList{ - create(), - }); - - auto* case_1_body = create(ast::StatementList{ - create(Expr(true), if_body, ast::ElseStatementList{}), - create(Expr("v"), Expr(1))}); - - ast::CaseSelectorList selector_1; - selector_1.push_back(Literal(1)); - - ast::CaseStatementList cases; - cases.push_back(create(selector_1, case_1_body)); - - auto* expr = create(Expr("a"), cases); + auto* expr = + Switch("a", /**/ + Case(Literal(1), + Block(/**/ + If(Expr(true), Block(create())), + Assign("v", 1))), + DefaultCase()); WrapInFunction(expr); diff --git a/test/BUILD.gn b/test/BUILD.gn index e6bc2611e8..d3d94bea46 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -170,6 +170,7 @@ source_set("tint_unittests_core_src") { "../src/program_test.cc", "../src/resolver/assignment_validation_test.cc", "../src/resolver/builtins_validation_test.cc", + "../src/resolver/control_block_validation_test.cc", "../src/resolver/decoration_validation_test.cc", "../src/resolver/function_validation_test.cc", "../src/resolver/host_shareable_validation_test.cc", @@ -218,7 +219,6 @@ source_set("tint_unittests_core_src") { "../src/utils/tmpfile.h", "../src/utils/tmpfile_test.cc", "../src/utils/unique_vector_test.cc", - "../src/validator/validator_control_block_test.cc", "../src/validator/validator_decoration_test.cc", "../src/validator/validator_function_test.cc", "../src/validator/validator_test.cc",