Move identifier validation from Validator to Resolver
This was mostly already implemented in the Resolver, except for adding a
variable scope for blocks.
Moved tests and improved them to only add Source on the error node.
Bug: tint:642
Change-Id: I175dd22c873df5933133bc92276101aeab3021ed
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/45460
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index 00cf333..3b43fc1 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -1726,7 +1726,10 @@
bool Resolver::BlockScope(BlockInfo::Type type, F&& callback) {
BlockInfo block_info(type, current_block_);
ScopedAssignment<BlockInfo*> sa(current_block_, &block_info);
- return callback();
+ variable_stack_.push_scope();
+ bool result = callback();
+ variable_stack_.pop_scope();
+ return result;
}
std::string Resolver::VectorPretty(uint32_t size, type::Type* element_type) {
diff --git a/src/resolver/validation_test.cc b/src/resolver/validation_test.cc
index f9366e3..30c5ba9 100644
--- a/src/resolver/validation_test.cc
+++ b/src/resolver/validation_test.cc
@@ -211,8 +211,7 @@
TEST_F(ResolverValidationTest, UsingUndefinedVariable_Fail) {
// b = 2;
- SetSource(Source{Source::Location{12, 34}});
- auto* lhs = Expr("b");
+ auto* lhs = Expr(Source{{12, 34}}, "b");
auto* rhs = Expr(2);
auto* assign = create<ast::AssignmentStatement>(lhs, rhs);
WrapInFunction(assign);
@@ -227,8 +226,7 @@
// b = 2;
// }
- SetSource(Source{Source::Location{12, 34}});
- auto* lhs = Expr("b");
+ auto* lhs = Expr(Source{{12, 34}}, "b");
auto* rhs = Expr(2);
auto* body = create<ast::BlockStatement>(ast::StatementList{
@@ -241,6 +239,133 @@
"12:34 error: v-0006: identifier must be declared before use: b");
}
+TEST_F(ResolverValidationTest, UsingUndefinedVariableGlobalVariableAfter_Fail) {
+ // fn my_func() -> void {
+ // global_var = 3.14f;
+ // }
+ // var global_var: f32 = 2.1;
+
+ auto* lhs = Expr(Source{{12, 34}}, "global_var");
+ auto* rhs = Expr(3.14f);
+
+ Func("my_func", ast::VariableList{}, ty.void_(),
+ ast::StatementList{
+ create<ast::AssignmentStatement>(lhs, rhs),
+ },
+ ast::DecorationList{
+ create<ast::StageDecoration>(ast::PipelineStage::kVertex)});
+
+ Global("global_var", ty.f32(), ast::StorageClass::kPrivate, Expr(2.1f));
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(),
+ "12:34 error: v-0006: identifier must be declared before use: "
+ "global_var");
+}
+
+TEST_F(ResolverValidationTest, UsingUndefinedVariableGlobalVariable_Pass) {
+ // var global_var: f32 = 2.1;
+ // fn my_func() -> void {
+ // global_var = 3.14;
+ // return;
+ // }
+
+ Global("global_var", ty.f32(), ast::StorageClass::kPrivate, Expr(2.1f));
+
+ Func("my_func", ast::VariableList{}, ty.void_(),
+ ast::StatementList{
+ create<ast::AssignmentStatement>(Source{Source::Location{12, 34}},
+ Expr("global_var"), Expr(3.14f)),
+ create<ast::ReturnStatement>(),
+ },
+ ast::DecorationList{
+ create<ast::StageDecoration>(ast::PipelineStage::kVertex),
+ });
+
+ EXPECT_TRUE(r()->Resolve()) << r()->error();
+}
+
+TEST_F(ResolverValidationTest, UsingUndefinedVariableInnerScope_Fail) {
+ // {
+ // if (true) { var a : f32 = 2.0; }
+ // a = 3.14;
+ // }
+ auto* var = Var("a", ty.f32(), ast::StorageClass::kNone, Expr(2.0f));
+
+ auto* cond = Expr(true);
+ auto* body = create<ast::BlockStatement>(ast::StatementList{
+ create<ast::VariableDeclStatement>(var),
+ });
+
+ SetSource(Source{Source::Location{12, 34}});
+ auto* lhs = Expr(Source{{12, 34}}, "a");
+ auto* rhs = Expr(3.14f);
+
+ auto* outer_body = create<ast::BlockStatement>(ast::StatementList{
+ create<ast::IfStatement>(cond, body, ast::ElseStatementList{}),
+ create<ast::AssignmentStatement>(lhs, rhs),
+ });
+
+ WrapInFunction(outer_body);
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(),
+ "12:34 error: v-0006: identifier must be declared before use: a");
+}
+
+TEST_F(ResolverValidationTest, UsingUndefinedVariableOuterScope_Pass) {
+ // {
+ // var a : f32 = 2.0;
+ // if (true) { a = 3.14; }
+ // }
+ auto* var = Var("a", ty.f32(), ast::StorageClass::kNone, Expr(2.0f));
+
+ auto* lhs = Expr(Source{{12, 34}}, "a");
+ auto* rhs = Expr(3.14f);
+
+ auto* cond = Expr(true);
+ auto* body = create<ast::BlockStatement>(ast::StatementList{
+ create<ast::AssignmentStatement>(lhs, rhs),
+ });
+
+ auto* outer_body = create<ast::BlockStatement>(ast::StatementList{
+ create<ast::VariableDeclStatement>(var),
+ create<ast::IfStatement>(cond, body, ast::ElseStatementList{}),
+ });
+
+ WrapInFunction(outer_body);
+
+ EXPECT_TRUE(r()->Resolve()) << r()->error();
+}
+
+TEST_F(ResolverValidationTest, UsingUndefinedVariableDifferentScope_Fail) {
+ // {
+ // { var a : f32 = 2.0; }
+ // { a = 3.14; }
+ // }
+ auto* var = Var("a", ty.f32(), ast::StorageClass::kNone, Expr(2.0f));
+ auto* first_body = create<ast::BlockStatement>(ast::StatementList{
+ create<ast::VariableDeclStatement>(var),
+ });
+
+ auto* lhs = Expr(Source{{12, 34}}, "a");
+ auto* rhs = Expr(3.14f);
+ auto* second_body = create<ast::BlockStatement>(ast::StatementList{
+ create<ast::AssignmentStatement>(lhs, rhs),
+ });
+
+ auto* outer_body = create<ast::BlockStatement>(ast::StatementList{
+ first_body,
+ second_body,
+ });
+
+ WrapInFunction(outer_body);
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(),
+ "12:34 error: v-0006: identifier must be declared before use: a");
+}
+
TEST_F(ResolverValidationTest, StorageClass_NonFunctionClassError) {
auto* var = Var("var", ty.i32(), ast::StorageClass::kWorkgroup);
diff --git a/src/validator/validator_impl.cc b/src/validator/validator_impl.cc
index 05e7f9e..021456c 100644
--- a/src/validator/validator_impl.cc
+++ b/src/validator/validator_impl.cc
@@ -411,11 +411,10 @@
return false;
}
} else {
- // The identifier is not defined. This should already have been caught
- // when validating the subexpression.
- add_error(ident->source(), "v-0006",
- "'" + program_->Symbols().NameFor(ident->symbol()) +
- "' is not declared");
+ // The identifier is not defined.
+ // Shouldn't reach here. This should already have been caught when
+ // validation expressions in the Resolver
+ TINT_UNREACHABLE(diagnostics());
return false;
}
return true;
@@ -463,9 +462,6 @@
if (!expr) {
return false;
}
- if (auto* i = expr->As<ast::IdentifierExpression>()) {
- return ValidateIdentifier(i);
- }
if (auto* c = expr->As<ast::CallExpression>()) {
return ValidateCallExpr(c);
@@ -473,17 +469,6 @@
return true;
}
-bool ValidatorImpl::ValidateIdentifier(const ast::IdentifierExpression* ident) {
- const ast::Variable* var;
- if (!variable_stack_.get(ident->symbol(), &var)) {
- add_error(ident->source(), "v-0006",
- "'" + program_->Symbols().NameFor(ident->symbol()) +
- "' is not declared");
- return false;
- }
- return true;
-}
-
bool ValidatorImpl::IsStorable(type::Type* type) {
if (type == nullptr) {
return false;
diff --git a/src/validator/validator_impl.h b/src/validator/validator_impl.h
index 93c1d8d..102fb4b 100644
--- a/src/validator/validator_impl.h
+++ b/src/validator/validator_impl.h
@@ -98,10 +98,6 @@
/// @param expr the expression to check
/// @return true if the expression is valid
bool ValidateExpression(const ast::Expression* expr);
- /// Validates v-0006:Variables must be defined before use
- /// @param ident the identifer to check if its in the scope
- /// @return true if idnet was defined
- bool ValidateIdentifier(const ast::IdentifierExpression* ident);
/// Validates declaration name uniqueness
/// @param decl is the new declaration to be added
/// @returns true if no previous declaration with the `decl` 's name
diff --git a/src/validator/validator_test.cc b/src/validator/validator_test.cc
index 84ce862..b5d8dba 100644
--- a/src/validator/validator_test.cc
+++ b/src/validator/validator_test.cc
@@ -185,155 +185,6 @@
EXPECT_FALSE(v.Validate()) << v.error();
}
-TEST_F(ValidatorTest, UsingUndefinedVariableGlobalVariableAfter_Fail) {
- // fn my_func() -> void {
- // global_var = 3.14f;
- // }
- // var global_var: f32 = 2.1;
-
- SetSource(Source{Source::Location{12, 34}});
- auto* lhs = Expr("global_var");
- auto* rhs = Expr(3.14f);
-
- Func("my_func", ast::VariableList{}, ty.void_(),
- ast::StatementList{
- create<ast::AssignmentStatement>(lhs, rhs),
- },
- ast::DecorationList{
- create<ast::StageDecoration>(ast::PipelineStage::kVertex)});
-
- Global("global_var", ty.f32(), ast::StorageClass::kPrivate, Expr(2.1f));
-
- // TODO(amaiorano): Move to resolver tests. Program is invalid now because
- // Resolver catches this. ValidatorImpl& v = Build();
-
- // EXPECT_FALSE(v.Validate());
- // EXPECT_EQ(v.error(), "12:34 v-0006: 'global_var' is not declared");
-}
-
-TEST_F(ValidatorTest, UsingUndefinedVariableGlobalVariable_Pass) {
- // var global_var: f32 = 2.1;
- // fn my_func() -> void {
- // global_var = 3.14;
- // return;
- // }
-
- Global("global_var", ty.f32(), ast::StorageClass::kPrivate, Expr(2.1f));
-
- Func("my_func", ast::VariableList{}, ty.void_(),
- ast::StatementList{
- create<ast::AssignmentStatement>(Source{Source::Location{12, 34}},
- Expr("global_var"), Expr(3.14f)),
- create<ast::ReturnStatement>(),
- },
- ast::DecorationList{
- create<ast::StageDecoration>(ast::PipelineStage::kVertex),
- });
-
- ValidatorImpl& v = Build();
-
- EXPECT_TRUE(v.Validate()) << v.error();
-}
-
-TEST_F(ValidatorTest, UsingUndefinedVariableInnerScope_Fail) {
- // {
- // if (true) { var a : f32 = 2.0; }
- // a = 3.14;
- // }
- auto* var = Var("a", ty.f32(), ast::StorageClass::kNone, Expr(2.0f));
-
- auto* cond = Expr(true);
- auto* body = create<ast::BlockStatement>(ast::StatementList{
- create<ast::VariableDeclStatement>(var),
- });
-
- SetSource(Source{Source::Location{12, 34}});
- auto* lhs = Expr("a");
- auto* rhs = Expr(3.14f);
-
- auto* outer_body = create<ast::BlockStatement>(ast::StatementList{
- create<ast::IfStatement>(cond, body, ast::ElseStatementList{}),
- create<ast::AssignmentStatement>(Source{Source::Location{12, 34}}, lhs,
- rhs),
- });
-
- WrapInFunction(outer_body);
-
- ValidatorImpl& v = Build();
-
- ASSERT_NE(TypeOf(lhs), nullptr);
- ASSERT_NE(TypeOf(rhs), nullptr);
-
- EXPECT_FALSE(v.ValidateStatements(outer_body));
- EXPECT_EQ(v.error(), "12:34 v-0006: 'a' is not declared");
-}
-
-TEST_F(ValidatorTest, UsingUndefinedVariableOuterScope_Pass) {
- // {
- // var a : f32 = 2.0;
- // if (true) { a = 3.14; }
- // }
- auto* var = Var("a", ty.f32(), ast::StorageClass::kNone, Expr(2.0f));
-
- SetSource(Source{Source::Location{12, 34}});
- auto* lhs = Expr("a");
- auto* rhs = Expr(3.14f);
-
- auto* cond = Expr(true);
- auto* body = create<ast::BlockStatement>(ast::StatementList{
- create<ast::AssignmentStatement>(Source{Source::Location{12, 34}}, lhs,
- rhs),
- });
-
- auto* outer_body = create<ast::BlockStatement>(ast::StatementList{
- create<ast::VariableDeclStatement>(var),
- create<ast::IfStatement>(cond, body, ast::ElseStatementList{}),
- });
-
- WrapInFunction(outer_body);
-
- ValidatorImpl& v = Build();
-
- ASSERT_NE(TypeOf(lhs), nullptr);
- ASSERT_NE(TypeOf(rhs), nullptr);
-
- EXPECT_TRUE(v.ValidateStatements(outer_body)) << v.error();
-}
-
-TEST_F(ValidatorTest, UsingUndefinedVariableDifferentScope_Fail) {
- // {
- // { var a : f32 = 2.0; }
- // { a = 3.14; }
- // }
- auto* var = Var("a", ty.f32(), ast::StorageClass::kNone, Expr(2.0f));
- auto* first_body = create<ast::BlockStatement>(ast::StatementList{
- create<ast::VariableDeclStatement>(var),
- });
-
- SetSource(Source{Source::Location{12, 34}});
- auto* lhs = Expr("a");
- auto* rhs = Expr(3.14f);
- auto* second_body = create<ast::BlockStatement>(ast::StatementList{
- create<ast::AssignmentStatement>(Source{Source::Location{12, 34}}, lhs,
- rhs),
- });
-
- auto* outer_body = create<ast::BlockStatement>(ast::StatementList{
- first_body,
- second_body,
- });
-
- WrapInFunction(outer_body);
-
- ValidatorImpl& v = Build();
-
- ASSERT_NE(TypeOf(lhs), nullptr);
- ASSERT_NE(TypeOf(rhs), nullptr);
-
- EXPECT_FALSE(v.ValidateStatements(outer_body));
- EXPECT_EQ(v.error(), "12:34 v-0006: 'a' is not declared");
-}
-
TEST_F(ValidatorTest, GlobalVariableUnique_Pass) {
// var global_var0 : f32 = 0.1;
// var global_var1 : i32 = 0;
