validation: cleaning up name uniqueness rules

Bug: tint:353
Change-Id: I01e5b63937e218d7c42e41f7c0e9a7910aacaa90
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/56320
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Auto-Submit: Sarah Mashayekhi <sarahmashay@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
This commit is contained in:
Sarah 2021-06-29 21:30:37 +00:00 committed by Sarah Mashayekhi
parent 288c8ad32e
commit c035366cd5
4 changed files with 71 additions and 58 deletions

View File

@ -42,8 +42,8 @@ TEST_F(ResolverFunctionValidationTest, FunctionNamesMustBeUnique_fail) {
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
R"(12:34 error: duplicate function named 'func'
note: first function declared here)");
"12:34 error: redefinition of 'func'\nnote: previous definition "
"is here");
}
TEST_F(ResolverFunctionValidationTest,
@ -74,8 +74,8 @@ TEST_F(ResolverFunctionValidationTest,
EXPECT_FALSE(r()->Resolve()) << r()->error();
EXPECT_EQ(r()->error(),
"12:34 error: duplicate declaration 'foo'\n56:78 note: 'foo' first "
"declared here:");
"12:34 error: redefinition of 'foo'\n56:78 note: previous "
"definition is here");
}
TEST_F(ResolverFunctionValidationTest,
@ -85,14 +85,14 @@ TEST_F(ResolverFunctionValidationTest,
Func(Source{Source::Location{12, 34}}, "foo", ast::VariableList{}, ty.void_(),
ast::StatementList{}, ast::DecorationList{});
auto* global_var =
Var("foo", ty.f32(), ast::StorageClass::kPrivate, Expr(3.14f));
auto* global_var = Var(Source{Source::Location{56, 78}}, "foo", ty.f32(),
ast::StorageClass::kPrivate, Expr(3.14f));
AST().AddGlobalVariable(global_var);
EXPECT_FALSE(r()->Resolve()) << r()->error();
EXPECT_EQ(r()->error(),
"error: duplicate declaration 'foo'\n12:34 note: 'foo' first "
"declared here:");
"56:78 error: redefinition of 'foo'\n12:34 note: previous "
"definition is here");
}
TEST_F(ResolverFunctionValidationTest, FunctionUsingSameVariableName_Pass) {

View File

@ -615,10 +615,8 @@ bool Resolver::ValidateVariableConstructor(const ast::Variable* var,
}
bool Resolver::GlobalVariable(ast::Variable* var) {
if (variable_stack_.has(var->symbol())) {
AddError("redeclared global identifier '" +
builder_->Symbols().NameFor(var->symbol()) + "'",
var->source());
if (!ValidateNoDuplicateDefinition(var->symbol(), var->source(),
/* check_global_scope_only */ true)) {
return false;
}
@ -673,17 +671,6 @@ bool Resolver::GlobalVariable(ast::Variable* var) {
}
bool Resolver::ValidateGlobalVariable(const VariableInfo* info) {
auto duplicate_func = symbol_to_function_.find(info->declaration->symbol());
if (duplicate_func != symbol_to_function_.end()) {
AddError("duplicate declaration '" +
builder_->Symbols().NameFor(info->declaration->symbol()) + "'",
info->declaration->source());
AddNote("'" + builder_->Symbols().NameFor(info->declaration->symbol()) +
"' first declared here:",
duplicate_func->second->declaration->source());
return false;
}
if (!ValidateNoDuplicateDecorations(info->declaration->decorations())) {
return false;
}
@ -1063,30 +1050,11 @@ bool Resolver::ValidateInterpolateDecoration(
bool Resolver::ValidateFunction(const ast::Function* func,
const FunctionInfo* info) {
auto func_it = symbol_to_function_.find(func->symbol());
if (func_it != symbol_to_function_.end()) {
AddError("duplicate function named '" +
builder_->Symbols().NameFor(func->symbol()) + "'",
func->source());
AddNote("first function declared here",
func_it->second->declaration->source());
if (!ValidateNoDuplicateDefinition(func->symbol(), func->source(),
/* check_global_scope_only */ true)) {
return false;
}
bool is_global = false;
VariableInfo* var;
if (variable_stack_.get(func->symbol(), &var, &is_global)) {
if (is_global) {
AddError("duplicate declaration '" +
builder_->Symbols().NameFor(func->symbol()) + "'",
func->source());
AddNote("'" + builder_->Symbols().NameFor(func->symbol()) +
"' first declared here:",
var->declaration->source());
return false;
}
}
auto stage_deco_count = 0;
auto workgroup_deco_count = 0;
for (auto* deco : func->decorations()) {
@ -2857,11 +2825,7 @@ bool Resolver::VariableDeclStatement(const ast::VariableDeclStatement* stmt) {
ast::Variable* var = stmt->variable();
Mark(var);
bool is_global = false;
if (variable_stack_.get(var->symbol(), nullptr, &is_global)) {
AddError("redeclared identifier '" +
builder_->Symbols().NameFor(var->symbol()) + "'",
var->source());
if (!ValidateNoDuplicateDefinition(var->symbol(), var->source())) {
return false;
}
@ -3772,6 +3736,39 @@ bool Resolver::ValidateAssignment(const ast::AssignmentStatement* a) {
return true;
}
bool Resolver::ValidateNoDuplicateDefinition(Symbol sym,
const Source& source,
bool check_global_scope_only) {
if (check_global_scope_only) {
bool is_global = false;
VariableInfo* var;
if (variable_stack_.get(sym, &var, &is_global)) {
if (is_global) {
AddError("redefinition of '" + builder_->Symbols().NameFor(sym) + "'",
source);
AddNote("previous definition is here", var->declaration->source());
return false;
}
}
auto it = symbol_to_function_.find(sym);
if (it != symbol_to_function_.end()) {
AddError("redefinition of '" + builder_->Symbols().NameFor(sym) + "'",
source);
AddNote("previous definition is here", it->second->declaration->source());
return false;
}
} else {
VariableInfo* var;
if (variable_stack_.get(sym, &var)) {
AddError("redefinition of '" + builder_->Symbols().NameFor(sym) + "'",
source);
AddNote("previous definition is here", var->declaration->source());
return false;
}
}
return true;
}
bool Resolver::ValidateNoDuplicateDecorations(
const ast::DecorationList& decorations) {
std::unordered_map<const TypeInfo*, Source> seen;

View File

@ -283,6 +283,10 @@ class Resolver {
const sem::Matrix* matrix_type);
bool ValidateFunctionParameter(const ast::Function* func,
const VariableInfo* info);
bool ValidateNoDuplicateDefinition(Symbol sym,
const Source& source,
bool check_global_scope_only = false);
bool ValidateParameter(const ast::Function* func, const VariableInfo* info);
bool ValidateReturn(const ast::ReturnStatement* ret);
bool ValidateStatements(const ast::StatementList& stmts);
bool ValidateStorageTexture(const ast::StorageTexture* t);

View File

@ -124,7 +124,9 @@ TEST_F(ResolverVarLetValidationTest, LocalVarRedeclared) {
WrapInFunction(v1, v2);
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: redeclared identifier 'v'");
EXPECT_EQ(
r()->error(),
"12:34 error: redefinition of 'v'\nnote: previous definition is here");
}
TEST_F(ResolverVarLetValidationTest, LocalLetRedeclared) {
@ -135,7 +137,9 @@ TEST_F(ResolverVarLetValidationTest, LocalLetRedeclared) {
WrapInFunction(l1, l2);
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: redeclared identifier 'l'");
EXPECT_EQ(
r()->error(),
"12:34 error: redefinition of 'l'\nnote: previous definition is here");
}
TEST_F(ResolverVarLetValidationTest, GlobalVarRedeclared) {
@ -145,8 +149,9 @@ TEST_F(ResolverVarLetValidationTest, GlobalVarRedeclared) {
Global(Source{{12, 34}}, "v", ty.i32(), ast::StorageClass::kPrivate);
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 error: redeclared global identifier 'v'");
EXPECT_EQ(
r()->error(),
"12:34 error: redefinition of 'v'\nnote: previous definition is here");
}
TEST_F(ResolverVarLetValidationTest, GlobalLetRedeclared) {
@ -156,8 +161,9 @@ TEST_F(ResolverVarLetValidationTest, GlobalLetRedeclared) {
GlobalConst(Source{{12, 34}}, "l", ty.i32(), Expr(0));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 error: redeclared global identifier 'l'");
EXPECT_EQ(
r()->error(),
"12:34 error: redefinition of 'l'\nnote: previous definition is here");
}
TEST_F(ResolverVarLetValidationTest, GlobalVarRedeclaredAsLocal) {
@ -173,7 +179,9 @@ TEST_F(ResolverVarLetValidationTest, GlobalVarRedeclaredAsLocal) {
Expr(2.0f)));
EXPECT_FALSE(r()->Resolve()) << r()->error();
EXPECT_EQ(r()->error(), "12:34 error: redeclared identifier 'v'");
EXPECT_EQ(
r()->error(),
"12:34 error: redefinition of 'v'\nnote: previous definition is here");
}
TEST_F(ResolverVarLetValidationTest, VarRedeclaredInInnerBlock) {
@ -190,7 +198,9 @@ TEST_F(ResolverVarLetValidationTest, VarRedeclaredInInnerBlock) {
WrapInFunction(outer_body);
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: redeclared identifier 'v'");
EXPECT_EQ(
r()->error(),
"12:34 error: redefinition of 'v'\nnote: previous definition is here");
}
TEST_F(ResolverVarLetValidationTest, VarRedeclaredInIfBlock) {
@ -213,7 +223,9 @@ TEST_F(ResolverVarLetValidationTest, VarRedeclaredInIfBlock) {
WrapInFunction(outer_body);
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: redeclared identifier 'v'");
EXPECT_EQ(
r()->error(),
"12:34 error: redefinition of 'v'\nnote: previous definition is here");
}
TEST_F(ResolverVarLetValidationTest, InferredPtrStorageAccessMismatch) {