encounter
/
dawn-cmake
mirror of https://github.com/encounter/dawn-cmake.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

resolver: Don't use ast to_str() methods 

These return weird names that love the use of '__' and have little relation to WGSL.

Improve the duplicate case error message.

Clean up control_block_validation_test.cc by making used of the ProgramBuilder helpers.

Bug: tint:1225
Change-Id: I8c4cf3943145cf8372c00d33ae0166c0c0bcbb8b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/66442
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
This commit is contained in:
Ben Clayton 2021-10-14 20:30:29 +00:00 committed by Tint LUCI CQ
parent 262210f12b
commit 11ed0db30d
4 changed files with 156 additions and 164 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

65
src/program_builder.h
View File

@ -1105,18 +1105,46 @@ class ProgramBuilder {
/// @return `list`
ast::ExpressionList ExprList(ast::ExpressionList list) { return list; }
/// @param source the source location for the literal
/// @param val the boolan value
/// @return a boolean literal with the given value
ast::BoolLiteral* Literal(const Source& source, bool val) {
return create<ast::BoolLiteral>(source, val);
}
/// @param val the boolan value
/// @return a boolean literal with the given value
ast::BoolLiteral* Literal(bool val) { return create<ast::BoolLiteral>(val); }
/// @param source the source location for the literal
/// @param val the float value
/// @return a float literal with the given value
ast::FloatLiteral* Literal(const Source& source, f32 val) {
return create<ast::FloatLiteral>(source, val);
}
/// @param val the float value
/// @return a float literal with the given value
ast::FloatLiteral* Literal(f32 val) { return create<ast::FloatLiteral>(val); }
/// @param source the source location for the literal
/// @param val the unsigned int value
/// @return a ast::UintLiteral with the given value
ast::UintLiteral* Literal(const Source& source, u32 val) {
return create<ast::UintLiteral>(source, val);
}
/// @param val the unsigned int value
/// @return a ast::UintLiteral with the given value
ast::UintLiteral* Literal(u32 val) { return create<ast::UintLiteral>(val); }
/// @param source the source location for the literal
/// @param val the integer value
/// @return the ast::SintLiteral with the given value
ast::SintLiteral* Literal(const Source& source, i32 val) {
return create<ast::SintLiteral>(source, val);
}
/// @param val the integer value
/// @return the ast::SintLiteral with the given value
ast::SintLiteral* Literal(i32 val) { return create<ast::SintLiteral>(val); }
@ -2071,16 +2099,44 @@ class ProgramBuilder {
}
/// Creates a ast::SwitchStatement with input expression and cases
/// @param source the source information
/// @param condition the condition expression initializer
/// @param cases case statements
/// @returns the switch statement pointer
template <typename ExpressionInit, typename... Cases>
ast::SwitchStatement* Switch(const Source& source,
ExpressionInit&& condition,
Cases&&... cases) {
return create<ast::SwitchStatement>(
source, Expr(std::forward<ExpressionInit>(condition)),
ast::CaseStatementList{std::forward<Cases>(cases)...});
}
/// 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 <typename ExpressionInit,
typename... Cases,
typename = DisableIfSource<ExpressionInit>>
ast::SwitchStatement* Switch(ExpressionInit&& condition, Cases&&... cases) {
return create<ast::SwitchStatement>(
Expr(std::forward<ExpressionInit>(condition)),
ast::CaseStatementList{std::forward<Cases>(cases)...});
}
/// Creates a ast::CaseStatement with input list of selectors, and body
/// @param source the source information
/// @param selectors list of selectors
/// @param body the case body
/// @returns the case statement pointer
ast::CaseStatement* Case(const Source& source,
ast::CaseSelectorList selectors,
ast::BlockStatement* body = nullptr) {
return create<ast::CaseStatement>(source, std::move(selectors),
body ? body : Block());
}
/// Creates a ast::CaseStatement with input list of selectors, and body
/// @param selectors list of selectors
/// @param body the case body
@ -2100,6 +2156,15 @@ class ProgramBuilder {
return Case(ast::CaseSelectorList{selector}, body);
}
/// Convenience function that creates a 'default' ast::CaseStatement
/// @param source the source information
/// @param body the case body
/// @returns the case statement pointer
ast::CaseStatement* DefaultCase(const Source& source,
ast::BlockStatement* body = nullptr) {
return Case(source, ast::CaseSelectorList{}, body);
}
/// Convenience function that creates a 'default' ast::CaseStatement
/// @param body the case body
/// @returns the case statement pointer

201
src/resolver/control_block_validation_test.cc
View File

@ -32,14 +32,8 @@ TEST_F(ResolverControlBlockValidationTest,
// }
auto* var = Var("a", ty.f32(), ast::StorageClass::kNone, Expr(3.14f));
ast::CaseStatementList body;
auto* block_default = Block();
body.push_back(
create<ast::CaseStatement>(ast::CaseSelectorList{}, block_default));
auto* block =
Block(Decl(var), create<ast::SwitchStatement>(
Expr(Source{Source::Location{12, 34}}, "a"), body));
auto* block = Block(Decl(var), Switch(Expr(Source{{12, 34}}, "a"), //
DefaultCase()));
WrapInFunction(block);
@ -56,15 +50,9 @@ TEST_F(ResolverControlBlockValidationTest, SwitchWithoutDefault_Fail) {
// }
auto* var = Var("a", ty.i32(), ast::StorageClass::kNone, Expr(2));
ast::CaseSelectorList csl;
csl.push_back(Literal(1));
ast::CaseStatementList body;
body.push_back(create<ast::CaseStatement>(csl, Block()));
auto* block =
Block(Decl(var), create<ast::SwitchStatement>(
Source{Source::Location{12, 34}}, Expr("a"), body));
auto* block = Block(Decl(var), //
Switch(Source{{12, 34}}, "a", //
Case(Literal(1))));
WrapInFunction(block);
@ -82,24 +70,11 @@ TEST_F(ResolverControlBlockValidationTest, SwitchWithTwoDefault_Fail) {
// }
auto* var = Var("a", ty.i32(), ast::StorageClass::kNone, Expr(2));
ast::CaseStatementList switch_body;
ast::CaseSelectorList default_csl_1;
auto* block_default_1 = Block();
switch_body.push_back(
create<ast::CaseStatement>(default_csl_1, block_default_1));
ast::CaseSelectorList csl_case_1;
csl_case_1.push_back(Literal(1));
auto* block_case_1 = Block();
switch_body.push_back(create<ast::CaseStatement>(csl_case_1, block_case_1));
ast::CaseSelectorList default_csl_2;
auto* block_default_2 = Block();
switch_body.push_back(create<ast::CaseStatement>(
Source{Source::Location{12, 34}}, default_csl_2, block_default_2));
auto* block =
Block(Decl(var), create<ast::SwitchStatement>(Expr("a"), switch_body));
auto* block = Block(Decl(var), //
Switch("a", //
DefaultCase(), //
Case(Literal(1)), //
DefaultCase(Source{{12, 34}})));
WrapInFunction(block);
@ -173,15 +148,16 @@ TEST_F(ResolverControlBlockValidationTest,
}
TEST_F(ResolverControlBlockValidationTest, UnreachableCode_break) {
// switch (a) {
// switch (1) {
// case 1: { break; var a : u32 = 2;}
// default: {}
// }
auto* decl = Decl(Source{{12, 34}},
Var("a", ty.i32(), ast::StorageClass::kNone, Expr(2)));
WrapInFunction(Loop(Block(Switch(
Expr(1), Case(Literal(1), Block(create<ast::BreakStatement>(), decl)),
WrapInFunction( //
Loop(Block(Switch(1, //
Case(Literal(1), Block(Break(), decl)), //
DefaultCase()))));
EXPECT_FALSE(r()->Resolve());
@ -189,17 +165,18 @@ TEST_F(ResolverControlBlockValidationTest, UnreachableCode_break) {
}
TEST_F(ResolverControlBlockValidationTest, UnreachableCode_break_InBlocks) {
// switch (a) {
// loop {
// switch (1) {
// case 1: { {{{break;}}} var a : u32 = 2;}
// default: {}
// }
// }
auto* decl = Decl(Source{{12, 34}},
Var("a", ty.i32(), ast::StorageClass::kNone, Expr(2)));
WrapInFunction(Loop(Block(Switch(
Expr(1),
Case(Literal(1),
Block(Block(Block(Block(create<ast::BreakStatement>()))), decl)),
WrapInFunction(Loop(
Block(Switch(1, //
Case(Literal(1), Block(Block(Block(Block(Break()))), decl)),
DefaultCase()))));
EXPECT_FALSE(r()->Resolve());
@ -215,18 +192,10 @@ TEST_F(ResolverControlBlockValidationTest,
// }
auto* var = Var("a", ty.i32(), ast::StorageClass::kNone, Expr(2));
ast::CaseStatementList switch_body;
ast::CaseSelectorList csl;
csl.push_back(create<ast::UintLiteral>(1u));
switch_body.push_back(create<ast::CaseStatement>(
Source{Source::Location{12, 34}}, csl, Block()));
ast::CaseSelectorList default_csl;
auto* block_default = Block();
switch_body.push_back(create<ast::CaseStatement>(default_csl, block_default));
auto* block =
Block(Decl(var), create<ast::SwitchStatement>(Expr("a"), switch_body));
Block(Decl(var), Switch("a", //
Case(Source{{12, 34}}, {Literal(1u)}), //
DefaultCase()));
WrapInFunction(block);
EXPECT_FALSE(r()->Resolve());
@ -244,18 +213,10 @@ TEST_F(ResolverControlBlockValidationTest,
// }
auto* var = Var("a", ty.u32(), ast::StorageClass::kNone, Expr(2u));
ast::CaseStatementList switch_body;
ast::CaseSelectorList csl;
csl.push_back(Literal(-1));
switch_body.push_back(create<ast::CaseStatement>(
Source{Source::Location{12, 34}}, csl, Block()));
ast::CaseSelectorList default_csl;
auto* block_default = Block();
switch_body.push_back(create<ast::CaseStatement>(default_csl, block_default));
auto* block =
Block(Decl(var), create<ast::SwitchStatement>(Expr("a"), switch_body));
auto* block = Block(Decl(var), //
Switch("a", //
Case(Source{{12, 34}}, {Literal(-1)}), //
DefaultCase()));
WrapInFunction(block);
EXPECT_FALSE(r()->Resolve());
@ -268,72 +229,55 @@ TEST_F(ResolverControlBlockValidationTest,
NonUniqueCaseSelectorValueUint_Fail) {
// var a : u32 = 3;
// switch (a) {
// case 0: {}
// case 2, 2: {}
// case 0u: {}
// case 2u, 3u, 2u: {}
// default: {}
// }
auto* var = Var("a", ty.u32(), ast::StorageClass::kNone, Expr(3u));
ast::CaseStatementList switch_body;
ast::CaseSelectorList csl_1;
csl_1.push_back(create<ast::UintLiteral>(0u));
switch_body.push_back(create<ast::CaseStatement>(csl_1, Block()));
ast::CaseSelectorList csl_2;
csl_2.push_back(create<ast::UintLiteral>(2u));
csl_2.push_back(create<ast::UintLiteral>(2u));
switch_body.push_back(create<ast::CaseStatement>(
Source{Source::Location{12, 34}}, csl_2, Block()));
ast::CaseSelectorList default_csl;
auto* block_default = Block();
switch_body.push_back(create<ast::CaseStatement>(default_csl, block_default));
auto* block =
Block(Decl(var), create<ast::SwitchStatement>(Expr("a"), switch_body));
auto* block = Block(Decl(var), //
Switch("a", //
Case(Literal(0u)),
Case({
Literal(Source{{12, 34}}, 2u),
Literal(3u),
Literal(Source{{56, 78}}, 2u),
}),
DefaultCase()));
WrapInFunction(block);
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 error: a literal value must not appear more than once in "
"the case selectors for a switch statement: '2u'");
"56:78 error: duplicate switch case '2'\n"
"12:34 note: previous case declared here");
}
TEST_F(ResolverControlBlockValidationTest,
NonUniqueCaseSelectorValueSint_Fail) {
// var a : i32 = 2;
// switch (a) {
// case 10: {}
// case 0,1,2,10: {}
// case -10: {}
// case 0,1,2,-10: {}
// default: {}
// }
auto* var = Var("a", ty.i32(), ast::StorageClass::kNone, Expr(2));
ast::CaseStatementList switch_body;
ast::CaseSelectorList csl_1;
csl_1.push_back(Literal(10));
switch_body.push_back(create<ast::CaseStatement>(csl_1, Block()));
ast::CaseSelectorList csl_2;
csl_2.push_back(Literal(0));
csl_2.push_back(Literal(1));
csl_2.push_back(Literal(2));
csl_2.push_back(Literal(10));
switch_body.push_back(create<ast::CaseStatement>(
Source{Source::Location{12, 34}}, csl_2, Block()));
ast::CaseSelectorList default_csl;
auto* block_default = Block();
switch_body.push_back(create<ast::CaseStatement>(default_csl, block_default));
auto* block =
Block(Decl(var), create<ast::SwitchStatement>(Expr("a"), switch_body));
auto* block = Block(Decl(var), //
Switch("a", //
Case(Literal(Source{{12, 34}}, -10)),
Case({
Literal(0),
Literal(1),
Literal(2),
Literal(Source{{56, 78}}, -10),
}),
DefaultCase()));
WrapInFunction(block);
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 error: a literal value must not appear more than once in "
"the case selectors for a switch statement: '10'");
"56:78 error: duplicate switch case '-10'\n"
"12:34 note: previous case declared here");
}
TEST_F(ResolverControlBlockValidationTest,
@ -343,14 +287,10 @@ TEST_F(ResolverControlBlockValidationTest,
// default: { fallthrough; }
// }
auto* var = Var("a", ty.i32(), ast::StorageClass::kNone, Expr(2));
ast::CaseSelectorList default_csl;
auto* block_default = Block(
create<ast::FallthroughStatement>(Source{Source::Location{12, 34}}));
ast::CaseStatementList body;
body.push_back(create<ast::CaseStatement>(default_csl, block_default));
auto* block = Block(Decl(var), create<ast::SwitchStatement>(Expr("a"), body));
auto* fallthrough = create<ast::FallthroughStatement>(Source{{12, 34}});
auto* block = Block(Decl(var), //
Switch("a", //
DefaultCase(Block(fallthrough))));
WrapInFunction(block);
EXPECT_FALSE(r()->Resolve());
@ -367,17 +307,10 @@ TEST_F(ResolverControlBlockValidationTest, SwitchCase_Pass) {
// }
auto* var = Var("a", ty.i32(), ast::StorageClass::kNone, Expr(2));
ast::CaseSelectorList default_csl;
auto* block_default = Block();
ast::CaseStatementList body;
body.push_back(create<ast::CaseStatement>(Source{Source::Location{12, 34}},
default_csl, block_default));
ast::CaseSelectorList case_csl;
case_csl.push_back(Literal(5));
auto* block_case = Block();
body.push_back(create<ast::CaseStatement>(case_csl, block_case));
auto* block = Block(Decl(var), create<ast::SwitchStatement>(Expr("a"), body));
auto* block = Block(Decl(var), //
Switch("a", //
DefaultCase(Source{{12, 34}}), //
Case(Literal(5))));
WrapInFunction(block);
EXPECT_TRUE(r()->Resolve()) << r()->error();
@ -392,14 +325,8 @@ TEST_F(ResolverControlBlockValidationTest, SwitchCaseAlias_Pass) {
auto* my_int = Alias("MyInt", ty.u32());
auto* var = Var("a", ty.Of(my_int), ast::StorageClass::kNone, Expr(2u));
ast::CaseSelectorList default_csl;
auto* block_default = Block();
ast::CaseStatementList body;
body.push_back(create<ast::CaseStatement>(Source{Source::Location{12, 34}},
default_csl, block_default));
auto* block = Block(Decl(var), create<ast::SwitchStatement>(Expr("a"), body));
auto* block = Block(Decl(var), //
Switch("a", DefaultCase(Source{{12, 34}})));
WrapInFunction(block);

26
src/resolver/resolver.cc
View File

@ -273,11 +273,9 @@ bool Resolver::ResolveInternal() {
bool result = true;
for (auto* node : builder_->ASTNodes().Objects()) {
if (marked_.count(node) == 0) {
TINT_ICE(Resolver, diagnostics_)
<< "AST node '" << node->TypeInfo().name
TINT_ICE(Resolver, diagnostics_) << "AST node '" << node->TypeInfo().name
<< "' was not reached by the resolver\n"
<< "At: " << node->source() << "\n"
<< "Content: " << builder_->str(node) << "\n"
<< "Pointer: " << node;
result = false;
}
@ -2123,8 +2121,8 @@ bool Resolver::Statement(ast::Statement* stmt) {
return VariableDeclStatement(v);
}
AddError(
"unknown statement type for type determination: " + builder_->str(stmt),
AddError("unknown statement type for type determination: " +
std::string(stmt->TypeInfo().name),
stmt->source());
return false;
}
@ -4389,7 +4387,7 @@ bool Resolver::ValidateSwitch(const ast::SwitchStatement* s) {
}
bool has_default = false;
std::unordered_set<uint32_t> selector_set;
std::unordered_map<uint32_t, Source> selectors;
for (auto* case_stmt : s->body()) {
if (case_stmt->IsDefault()) {
@ -4412,15 +4410,16 @@ bool Resolver::ValidateSwitch(const ast::SwitchStatement* s) {
}
auto v = selector->value_as_u32();
if (selector_set.find(v) != selector_set.end()) {
AddError(
"a literal value must not appear more than once in "
"the case selectors for a switch statement: '" +
builder_->str(selector) + "'",
case_stmt->source());
auto it = selectors.find(v);
if (it != selectors.end()) {
auto val = selector->Is<ast::IntLiteral>()
? std::to_string(selector->value_as_i32())
: std::to_string(selector->value_as_u32());
AddError("duplicate switch case '" + val + "'", selector->source());
AddNote("previous case declared here", it->second);
return false;
}
selector_set.emplace(v);
selectors.emplace(v, selector->source());
}
}
@ -4659,7 +4658,6 @@ void Resolver::Mark(const ast::Node* node) {
<< "AST node '" << node->TypeInfo().name
<< "' was encountered twice in the same AST of a Program\n"
<< "At: " << node->source() << "\n"
<< "Content: " << builder_->str(node) << "\n"
<< "Pointer: " << node;
}

12
src/resolver/validation_test.cc
View File

@ -47,10 +47,9 @@ namespace {
using ResolverValidationTest = ResolverTest;
class FakeStmt : public ast::Statement {
class FakeStmt : public Castable<FakeStmt, ast::Statement> {
public:
FakeStmt(ProgramID program_id, Source source)
: ast::Statement(program_id, source) {}
FakeStmt(ProgramID program_id, Source source) : Base(program_id, source) {}
FakeStmt* Clone(CloneContext*) const override { return nullptr; }
void to_str(const sem::Info&, std::ostream& out, size_t) const override {
out << "Fake";
@ -118,7 +117,8 @@ TEST_F(ResolverValidationTest, Error_WithEmptySource) {
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"error: unknown statement type for type determination: Fake");
"error: unknown statement type for type determination: "
"tint::resolver::FakeStmt");
}
TEST_F(ResolverValidationTest, Stmt_Error_Unknown) {
@ -128,7 +128,8 @@ TEST_F(ResolverValidationTest, Stmt_Error_Unknown) {
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"2:30 error: unknown statement type for type determination: Fake");
"2:30 error: unknown statement type for type determination: "
"tint::resolver::FakeStmt");
}
TEST_F(ResolverValidationTest, Stmt_If_NonBool) {
@ -1057,4 +1058,5 @@ TEST_F(ResolverTest, Expr_Constructor_Cast_Pointer) {
} // namespace resolver
} // namespace tint
TINT_INSTANTIATE_TYPEINFO(tint::resolver::FakeStmt);
TINT_INSTANTIATE_TYPEINFO(tint::resolver::FakeExpr);