[validation] Validates declaration name uniqueness
This CL adds implementations and tests for these validation rules:
v-0011: Global variable names must be unique
v-0013: Variables declared in a function must be unique between that function and any global variables.
v-0014: Variables declared in a function must have unique names
Bug: tint 6
Change-Id: I793485c981f67abc6a3dc81d35be743ccc18db5b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/26480
Reviewed-by: dan sinclair <dsinclair@chromium.org>
diff --git a/src/scope_stack.h b/src/scope_stack.h
index 5e8b57e..f127c97 100644
--- a/src/scope_stack.h
+++ b/src/scope_stack.h
@@ -64,14 +64,27 @@
/// @param ret where to place the name
/// @returns true if the name was successfully found, false otherwise
bool get(const std::string& name, T* ret) const {
+ return get(name, ret, nullptr);
+ }
+
+ /// Retrieves a given name from the stack
+ /// @param name the name to look for
+ /// @param ret where to place the name
+ /// @param is_global set true if the name references a global variable
+ /// otherwise unchanged
+ /// @returns true if the name was successfully found, false otherwise
+ bool get(const std::string& name, T* ret, bool* is_global) const {
for (auto iter = stack_.rbegin(); iter != stack_.rend(); ++iter) {
auto& map = *iter;
-
auto val = map.find(name);
+
if (val != map.end()) {
if (ret) {
*ret = val->second;
}
+ if (is_global && iter == stack_.rend() - 1) {
+ *is_global = true;
+ }
return true;
}
}
diff --git a/src/validator_impl.cc b/src/validator_impl.cc
index da7908c..b330196 100644
--- a/src/validator_impl.cc
+++ b/src/validator_impl.cc
@@ -31,6 +31,11 @@
return false;
}
for (const auto& var : module->global_variables()) {
+ if (variable_stack_.has(var->name())) {
+ set_error(var->source(),
+ "v-0011: redeclared global identifier '" + var->name() + "'");
+ return false;
+ }
variable_stack_.set_global(var->name(), var.get());
}
if (!CheckImports(module)) {
@@ -70,11 +75,6 @@
return false;
}
for (const auto& stmt : *block) {
- // TODO(sarahM0): move the folowing to a function
- if (stmt->IsVariableDecl()) {
- auto* v = stmt->AsVariableDecl();
- variable_stack_.set(v->variable()->name(), v->variable());
- }
if (!ValidateStatement(stmt.get())) {
return false;
}
@@ -82,10 +82,30 @@
return true;
}
+bool ValidatorImpl::ValidateDeclStatement(
+ const ast::VariableDeclStatement* decl) {
+ auto name = decl->variable()->name();
+ bool is_global = false;
+ if (variable_stack_.get(name, nullptr, &is_global)) {
+ std::string error_number = "v-0014: ";
+ if (is_global) {
+ error_number = "v-0013: ";
+ }
+ set_error(decl->source(),
+ error_number + "redeclared identifier '" + name + "'");
+ return false;
+ }
+ variable_stack_.set(name, decl->variable());
+ return true;
+}
+
bool ValidatorImpl::ValidateStatement(const ast::Statement* stmt) {
if (!stmt) {
return false;
}
+ if (stmt->IsVariableDecl()) {
+ return ValidateDeclStatement(stmt->AsVariableDecl());
+ }
if (stmt->IsAssign()) {
return ValidateAssign(stmt->AsAssign());
}
diff --git a/src/validator_impl.h b/src/validator_impl.h
index d7a7fc3..0019d7d 100644
--- a/src/validator_impl.h
+++ b/src/validator_impl.h
@@ -89,6 +89,11 @@
/// @param assign is the assigment to check if its lhs is a const
/// @returns false if lhs of assign is a constant identifier
bool ValidateConstant(const ast::AssignmentStatement* assign);
+ /// Validates declaration name uniquness
+ /// @param decl is the new declartion to be added
+ /// @returns true if no previous decleration with the |decl|'s name
+ /// exist in the variable stack
+ bool ValidateDeclStatement(const ast::VariableDeclStatement* decl);
private:
std::string error_;
diff --git a/src/validator_test.cc b/src/validator_test.cc
index cb5d6db..0a116b9 100644
--- a/src/validator_test.cc
+++ b/src/validator_test.cc
@@ -48,6 +48,7 @@
#include "src/ast/type/pointer_type.h"
#include "src/ast/type/struct_type.h"
#include "src/ast/type/vector_type.h"
+#include "src/ast/type/void_type.h"
#include "src/ast/type_constructor_expression.h"
#include "src/ast/variable.h"
#include "src/ast/variable_decl_statement.h"
@@ -240,7 +241,7 @@
ASSERT_NE(rhs_ptr->result_type(), nullptr);
tint::ValidatorImpl v;
- EXPECT_TRUE(v.ValidateStatements(body.get()));
+ EXPECT_TRUE(v.ValidateStatements(body.get())) << v.error();
}
TEST_F(ValidatorTest, AssignIncompatibleTypesInBlockStatement_Fail) {
@@ -278,7 +279,11 @@
"12:34: v-000x: invalid assignment of '__i32' to '__f32'");
}
-TEST_F(ValidatorTest, UnsingUndefinedVariableGlobalVariable_Fail) {
+TEST_F(ValidatorTest, UsingUndefinedVariableGlobalVariable_Fail) {
+ // var global_var: f32 = 2.1;
+ // fn my_func() -> f32 {
+ // not_global_var = 3.14f;
+ // }
ast::type::F32Type f32;
auto global_var = std::make_unique<ast::Variable>(
"global_var", ast::StorageClass::kPrivate, &f32);
@@ -307,7 +312,11 @@
EXPECT_EQ(v.error(), "12:34: v-0006: 'not_global_var' is not declared");
}
-TEST_F(ValidatorTest, UnsingUndefinedVariableGlobalVariable_Pass) {
+TEST_F(ValidatorTest, UsingUndefinedVariableGlobalVariable_Pass) {
+ // var global_var: f32 = 2.1;
+ // fn my_func() -> f32 {
+ // global_var = 3.14;
+ // }
ast::type::F32Type f32;
auto global_var = std::make_unique<ast::Variable>(
"global_var", ast::StorageClass::kPrivate, &f32);
@@ -341,10 +350,10 @@
EXPECT_TRUE(v.Validate(mod())) << v.error();
}
-TEST_F(ValidatorTest, UnsingUndefinedVariableInnerScope_Fail) {
+TEST_F(ValidatorTest, UsingUndefinedVariableInnerScope_Fail) {
// {
- // if (true) { var a : f32 = 2.0; }
- // a = 3.14;
+ // if (true) { var a : f32 = 2.0; }
+ // a = 3.14;
// }
ast::type::F32Type f32;
auto var =
@@ -378,10 +387,10 @@
EXPECT_EQ(v.error(), "12:34: v-0006: 'a' is not declared");
}
-TEST_F(ValidatorTest, UnsingUndefinedVariableOuterScope_Pass) {
+TEST_F(ValidatorTest, UsingUndefinedVariableOuterScope_Pass) {
// {
- // var a : f32 = 2.0;
- // if (true) { a = 3.14; }
+ // var a : f32 = 2.0;
+ // if (true) { a = 3.14; }
// }
ast::type::F32Type f32;
auto var =
@@ -413,6 +422,27 @@
EXPECT_TRUE(v.ValidateStatements(outer_body.get())) << v.error();
}
+TEST_F(ValidatorTest, GlobalVariableUnique_Pass) {
+ // var global_var0 : f32 = 0.1;
+ // var global_var1 : i32 = 0;
+ ast::type::F32Type f32;
+ ast::type::I32Type i32;
+ auto var0 = std::make_unique<ast::Variable>(
+ "global_var0", ast::StorageClass::kPrivate, &f32);
+ var0->set_constructor(std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::FloatLiteral>(&f32, 0.1)));
+ mod()->AddGlobalVariable(std::move(var0));
+
+ auto var1 = std::make_unique<ast::Variable>(
+ Source{12, 34}, "global_var1", ast::StorageClass::kPrivate, &f32);
+ var1->set_constructor(std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::SintLiteral>(&i32, 0)));
+ mod()->AddGlobalVariable(std::move(var1));
+
+ tint::ValidatorImpl v;
+ EXPECT_TRUE(v.Validate(mod())) << v.error();
+}
+
TEST_F(ValidatorTest, AssignToConstant_Fail) {
// {
// const a :i32 = 2;
@@ -445,5 +475,213 @@
EXPECT_EQ(v.error(), "12:34: v-0021: cannot re-assign a constant: 'a'");
}
+TEST_F(ValidatorTest, GlobalVariableNotUnique_Fail) {
+ // var global_var : f32 = 0.1;
+ // var global_var : i32 = 0;
+ ast::type::F32Type f32;
+ ast::type::I32Type i32;
+ auto var0 = std::make_unique<ast::Variable>(
+ "global_var", ast::StorageClass::kPrivate, &f32);
+ var0->set_constructor(std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::FloatLiteral>(&f32, 0.1)));
+ mod()->AddGlobalVariable(std::move(var0));
+
+ auto var1 = std::make_unique<ast::Variable>(
+ Source{12, 34}, "global_var", ast::StorageClass::kPrivate, &f32);
+ var1->set_constructor(std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::SintLiteral>(&i32, 0)));
+ mod()->AddGlobalVariable(std::move(var1));
+
+ tint::ValidatorImpl v;
+ EXPECT_FALSE(v.Validate(mod()));
+ EXPECT_EQ(v.error(),
+ "12:34: v-0011: redeclared global identifier 'global_var'");
+}
+
+TEST_F(ValidatorTest, GlobalVariableFunctionVariableNotUnique_Fail) {
+ // var a: f32 = 2.1;
+ // fn my_func -> void {
+ // var a: f32 = 2.0;
+ // return 0;
+ // }
+
+ ast::type::VoidType void_type;
+ ast::type::F32Type f32;
+ auto global_var =
+ std::make_unique<ast::Variable>("a", ast::StorageClass::kPrivate, &f32);
+ global_var->set_constructor(
+ std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::FloatLiteral>(&f32, 2.1)));
+ mod()->AddGlobalVariable(std::move(global_var));
+
+ auto var =
+ std::make_unique<ast::Variable>("a", ast::StorageClass::kNone, &f32);
+ var->set_constructor(std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::FloatLiteral>(&f32, 2.0)));
+ ast::VariableList params;
+ auto func =
+ std::make_unique<ast::Function>("my_func", std::move(params), &void_type);
+ auto body = std::make_unique<ast::BlockStatement>();
+ body->append(std::make_unique<ast::VariableDeclStatement>(Source{12, 34},
+ std::move(var)));
+ func->set_body(std::move(body));
+ auto* func_ptr = func.get();
+ mod()->AddFunction(std::move(func));
+
+ EXPECT_TRUE(td()->Determine()) << td()->error();
+ EXPECT_TRUE(td()->DetermineFunction(func_ptr)) << td()->error();
+ tint::ValidatorImpl v;
+ EXPECT_FALSE(v.Validate(mod())) << v.error();
+ EXPECT_EQ(v.error(), "12:34: v-0013: redeclared identifier 'a'");
+}
+
+TEST_F(ValidatorTest, RedeclaredIndentifier_Fail) {
+ // fn my_func() -> void {
+ // var a :i32 = 2;
+ // var a :f21 = 2.0;
+ // }
+ ast::type::VoidType void_type;
+ ast::type::I32Type i32;
+ ast::type::F32Type f32;
+ auto var =
+ std::make_unique<ast::Variable>("a", ast::StorageClass::kNone, &i32);
+ var->set_constructor(std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::SintLiteral>(&i32, 2)));
+
+ auto var_a_float =
+ std::make_unique<ast::Variable>("a", ast::StorageClass::kNone, &f32);
+ var_a_float->set_constructor(
+ std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::FloatLiteral>(&f32, 0.1)));
+
+ ast::VariableList params;
+ auto func =
+ std::make_unique<ast::Function>("my_func", std::move(params), &void_type);
+ auto body = std::make_unique<ast::BlockStatement>();
+ body->append(std::make_unique<ast::VariableDeclStatement>(std::move(var)));
+ body->append(std::make_unique<ast::VariableDeclStatement>(
+ Source{12, 34}, std::move(var_a_float)));
+ func->set_body(std::move(body));
+ auto* func_ptr = func.get();
+ mod()->AddFunction(std::move(func));
+
+ EXPECT_TRUE(td()->Determine()) << td()->error();
+ EXPECT_TRUE(td()->DetermineFunction(func_ptr)) << td()->error();
+ tint::ValidatorImpl v;
+ EXPECT_FALSE(v.Validate(mod()));
+ EXPECT_EQ(v.error(), "12:34: v-0014: redeclared identifier 'a'");
+}
+
+TEST_F(ValidatorTest, RedeclaredIdentifierInnerScope_Pass) {
+ // {
+ // if (true) { var a : f32 = 2.0; }
+ // var a : f32 = 3.14;
+ // }
+ ast::type::F32Type f32;
+ auto var =
+ std::make_unique<ast::Variable>("a", ast::StorageClass::kNone, &f32);
+ var->set_constructor(std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::FloatLiteral>(&f32, 2.0)));
+
+ ast::type::BoolType bool_type;
+ auto cond = std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::BoolLiteral>(&bool_type, true));
+ auto body = std::make_unique<ast::BlockStatement>();
+ body->append(std::make_unique<ast::VariableDeclStatement>(std::move(var)));
+
+ auto var_a_float =
+ std::make_unique<ast::Variable>("a", ast::StorageClass::kNone, &f32);
+ var_a_float->set_constructor(
+ std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::FloatLiteral>(&f32, 3.14)));
+
+ auto outer_body = std::make_unique<ast::BlockStatement>();
+ outer_body->append(
+ std::make_unique<ast::IfStatement>(std::move(cond), std::move(body)));
+ outer_body->append(std::make_unique<ast::VariableDeclStatement>(
+ Source{12, 34}, std::move(var_a_float)));
+
+ EXPECT_TRUE(td()->DetermineStatements(outer_body.get())) << td()->error();
+ tint::ValidatorImpl v;
+ EXPECT_TRUE(v.ValidateStatements(outer_body.get())) << v.error();
+}
+
+TEST_F(ValidatorTest, DISABLED_RedeclaredIdentifierInnerScope_False) {
+ // TODO(sarahM0): remove DISABLED after implementing ValidateIfStatement
+ // and it should just work
+ // {
+ // var a : f32 = 3.14;
+ // if (true) { var a : f32 = 2.0; }
+ // }
+ ast::type::F32Type f32;
+ auto var_a_float =
+ std::make_unique<ast::Variable>("a", ast::StorageClass::kNone, &f32);
+ var_a_float->set_constructor(
+ std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::FloatLiteral>(&f32, 3.14)));
+
+ auto var =
+ std::make_unique<ast::Variable>("a", ast::StorageClass::kNone, &f32);
+ var->set_constructor(std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::FloatLiteral>(&f32, 2.0)));
+
+ ast::type::BoolType bool_type;
+ auto cond = std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::BoolLiteral>(&bool_type, true));
+ auto body = std::make_unique<ast::BlockStatement>();
+ body->append(std::make_unique<ast::VariableDeclStatement>(Source{12, 34},
+ std::move(var)));
+
+ auto outer_body = std::make_unique<ast::BlockStatement>();
+ outer_body->append(
+ std::make_unique<ast::VariableDeclStatement>(std::move(var_a_float)));
+ outer_body->append(
+ std::make_unique<ast::IfStatement>(std::move(cond), std::move(body)));
+
+ EXPECT_TRUE(td()->DetermineStatements(outer_body.get())) << td()->error();
+ tint::ValidatorImpl v;
+ EXPECT_FALSE(v.ValidateStatements(outer_body.get()));
+ EXPECT_EQ(v.error(), "12:34: v-0014: redeclared identifier 'a'");
+}
+
+TEST_F(ValidatorTest, RedeclaredIdentifierDifferentFunctions_Pass) {
+ // func0 { var a : f32 = 2.0; }
+ // func1 { var a : f32 = 3.0; }
+ ast::type::F32Type f32;
+ auto var0 =
+ std::make_unique<ast::Variable>("a", ast::StorageClass::kNone, &f32);
+ var0->set_constructor(std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::FloatLiteral>(&f32, 2.0)));
+
+ auto var1 =
+ std::make_unique<ast::Variable>("a", ast::StorageClass::kNone, &f32);
+ var1->set_constructor(std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::FloatLiteral>(&f32, 1.0)));
+
+ ast::VariableList params0;
+ auto func0 =
+ std::make_unique<ast::Function>("func0", std::move(params0), &f32);
+ auto body0 = std::make_unique<ast::BlockStatement>();
+ body0->append(std::make_unique<ast::VariableDeclStatement>(Source{12, 34},
+ std::move(var0)));
+ func0->set_body(std::move(body0));
+
+ ast::VariableList params1;
+ auto func1 =
+ std::make_unique<ast::Function>("func1", std::move(params1), &f32);
+ auto body1 = std::make_unique<ast::BlockStatement>();
+ body1->append(std::make_unique<ast::VariableDeclStatement>(Source{13, 34},
+ std::move(var1)));
+ func1->set_body(std::move(body1));
+
+ mod()->AddFunction(std::move(func0));
+ mod()->AddFunction(std::move(func1));
+
+ EXPECT_TRUE(td()->Determine()) << td()->error();
+ EXPECT_TRUE(td()->Determine()) << td()->error();
+ tint::ValidatorImpl v;
+ EXPECT_TRUE(v.Validate(mod())) << v.error();
+}
+
} // namespace
} // namespace tint
