From d3107bdbaa459e2d32882193230bf021c2c0d033 Mon Sep 17 00:00:00 2001 From: Sarah Mashayekhi Date: Tue, 11 Aug 2020 20:44:06 +0000 Subject: [PATCH] [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 --- src/scope_stack.h | 15 ++- src/validator_impl.cc | 30 ++++- src/validator_impl.h | 5 + src/validator_test.cc | 256 ++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 291 insertions(+), 15 deletions(-) diff --git a/src/scope_stack.h b/src/scope_stack.h index 5e8b57ef50..f127c97903 100644 --- a/src/scope_stack.h +++ b/src/scope_stack.h @@ -64,14 +64,27 @@ class ScopeStack { /// @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 da7908c436..b3301964d3 100644 --- a/src/validator_impl.cc +++ b/src/validator_impl.cc @@ -31,6 +31,11 @@ bool ValidatorImpl::Validate(const ast::Module* module) { 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 @@ bool ValidatorImpl::ValidateStatements(const ast::BlockStatement* block) { 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 @@ bool ValidatorImpl::ValidateStatements(const ast::BlockStatement* block) { 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 d7a7fc3227..0019d7da87 100644 --- a/src/validator_impl.h +++ b/src/validator_impl.h @@ -89,6 +89,11 @@ class ValidatorImpl { /// @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 cb5d6dbd63..0a116b9ea5 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 @@ TEST_F(ValidatorTest, AssignCompatibleTypesInBlockStatement_Pass) { 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 @@ TEST_F(ValidatorTest, AssignIncompatibleTypesInBlockStatement_Fail) { "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( "global_var", ast::StorageClass::kPrivate, &f32); @@ -307,7 +312,11 @@ TEST_F(ValidatorTest, UnsingUndefinedVariableGlobalVariable_Fail) { 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( "global_var", ast::StorageClass::kPrivate, &f32); @@ -341,10 +350,10 @@ TEST_F(ValidatorTest, UnsingUndefinedVariableGlobalVariable_Pass) { 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 @@ TEST_F(ValidatorTest, UnsingUndefinedVariableInnerScope_Fail) { 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 @@ TEST_F(ValidatorTest, UnsingUndefinedVariableOuterScope_Pass) { 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( + "global_var0", ast::StorageClass::kPrivate, &f32); + var0->set_constructor(std::make_unique( + std::make_unique(&f32, 0.1))); + mod()->AddGlobalVariable(std::move(var0)); + + auto var1 = std::make_unique( + Source{12, 34}, "global_var1", ast::StorageClass::kPrivate, &f32); + var1->set_constructor(std::make_unique( + std::make_unique(&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 @@ TEST_F(ValidatorTest, AssignToConstant_Fail) { 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( + "global_var", ast::StorageClass::kPrivate, &f32); + var0->set_constructor(std::make_unique( + std::make_unique(&f32, 0.1))); + mod()->AddGlobalVariable(std::move(var0)); + + auto var1 = std::make_unique( + Source{12, 34}, "global_var", ast::StorageClass::kPrivate, &f32); + var1->set_constructor(std::make_unique( + std::make_unique(&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("a", ast::StorageClass::kPrivate, &f32); + global_var->set_constructor( + std::make_unique( + std::make_unique(&f32, 2.1))); + mod()->AddGlobalVariable(std::move(global_var)); + + auto var = + std::make_unique("a", ast::StorageClass::kNone, &f32); + var->set_constructor(std::make_unique( + std::make_unique(&f32, 2.0))); + ast::VariableList params; + auto func = + std::make_unique("my_func", std::move(params), &void_type); + auto body = std::make_unique(); + body->append(std::make_unique(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("a", ast::StorageClass::kNone, &i32); + var->set_constructor(std::make_unique( + std::make_unique(&i32, 2))); + + auto var_a_float = + std::make_unique("a", ast::StorageClass::kNone, &f32); + var_a_float->set_constructor( + std::make_unique( + std::make_unique(&f32, 0.1))); + + ast::VariableList params; + auto func = + std::make_unique("my_func", std::move(params), &void_type); + auto body = std::make_unique(); + body->append(std::make_unique(std::move(var))); + body->append(std::make_unique( + 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("a", ast::StorageClass::kNone, &f32); + var->set_constructor(std::make_unique( + std::make_unique(&f32, 2.0))); + + ast::type::BoolType bool_type; + auto cond = std::make_unique( + std::make_unique(&bool_type, true)); + auto body = std::make_unique(); + body->append(std::make_unique(std::move(var))); + + auto var_a_float = + std::make_unique("a", ast::StorageClass::kNone, &f32); + var_a_float->set_constructor( + std::make_unique( + std::make_unique(&f32, 3.14))); + + auto outer_body = std::make_unique(); + outer_body->append( + std::make_unique(std::move(cond), std::move(body))); + outer_body->append(std::make_unique( + 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("a", ast::StorageClass::kNone, &f32); + var_a_float->set_constructor( + std::make_unique( + std::make_unique(&f32, 3.14))); + + auto var = + std::make_unique("a", ast::StorageClass::kNone, &f32); + var->set_constructor(std::make_unique( + std::make_unique(&f32, 2.0))); + + ast::type::BoolType bool_type; + auto cond = std::make_unique( + std::make_unique(&bool_type, true)); + auto body = std::make_unique(); + body->append(std::make_unique(Source{12, 34}, + std::move(var))); + + auto outer_body = std::make_unique(); + outer_body->append( + std::make_unique(std::move(var_a_float))); + outer_body->append( + std::make_unique(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("a", ast::StorageClass::kNone, &f32); + var0->set_constructor(std::make_unique( + std::make_unique(&f32, 2.0))); + + auto var1 = + std::make_unique("a", ast::StorageClass::kNone, &f32); + var1->set_constructor(std::make_unique( + std::make_unique(&f32, 1.0))); + + ast::VariableList params0; + auto func0 = + std::make_unique("func0", std::move(params0), &f32); + auto body0 = std::make_unique(); + body0->append(std::make_unique(Source{12, 34}, + std::move(var0))); + func0->set_body(std::move(body0)); + + ast::VariableList params1; + auto func1 = + std::make_unique("func1", std::move(params1), &f32); + auto body1 = std::make_unique(); + body1->append(std::make_unique(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