[validation] Handle variable scoping for nested blocks
* Push/pop the variable scope stack when validating blocks
* Handle nested blocks in ValidateStatement()
* Add test coverage
This also fixes issues with other types of validation errors not being
caught when inside nested blocks.
Change-Id: Ia8d0138b346a8a7aa607497d51fd6aaf675dc8be
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/39980
Auto-Submit: James Price <jrprice@google.com>
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
diff --git a/src/validator/validator_impl.cc b/src/validator/validator_impl.cc
index 73bb0f4..0b0b163 100644
--- a/src/validator/validator_impl.cc
+++ b/src/validator/validator_impl.cc
@@ -481,12 +481,18 @@
if (!block) {
return false;
}
+
+ bool is_valid = true;
+ variable_stack_.push_scope();
for (auto* stmt : *block) {
if (!ValidateStatement(stmt)) {
- return false;
+ is_valid = false;
+ break;
}
}
- return true;
+ variable_stack_.pop_scope();
+
+ return is_valid;
}
bool ValidatorImpl::ValidateDeclStatement(
@@ -546,6 +552,9 @@
if (auto* c = stmt->As<ast::CaseStatement>()) {
return ValidateCase(c);
}
+ if (auto* b = stmt->As<ast::BlockStatement>()) {
+ return ValidateStatements(b);
+ }
return true;
}
diff --git a/src/validator/validator_test.cc b/src/validator/validator_test.cc
index 278040c..1a0f972 100644
--- a/src/validator/validator_test.cc
+++ b/src/validator/validator_test.cc
@@ -326,6 +326,44 @@
"'__f32' to '__i32'");
}
+TEST_F(ValidatorTest, AssignIncompatibleTypesInNestedBlockStatement_Fail) {
+ // {
+ // {
+ // var a :i32 = 2;
+ // a = 2.3;
+ // }
+ // }
+
+ auto* var = Var("a", ast::StorageClass::kNone, ty.i32(), Expr(2),
+ ast::VariableDecorationList{});
+
+ auto* lhs = Expr("a");
+ auto* rhs = Expr(2.3f);
+
+ auto* inner_block = create<ast::BlockStatement>(ast::StatementList{
+ create<ast::VariableDeclStatement>(var),
+ create<ast::AssignmentStatement>(Source{Source::Location{12, 34}}, lhs,
+ rhs),
+ });
+
+ auto* outer_block = create<ast::BlockStatement>(ast::StatementList{
+ inner_block,
+ });
+
+ EXPECT_TRUE(td()->DetermineStatements(outer_block)) << td()->error();
+ ASSERT_NE(TypeOf(lhs), nullptr);
+ ASSERT_NE(TypeOf(rhs), nullptr);
+
+ ValidatorImpl& v = Build();
+
+ EXPECT_FALSE(v.ValidateStatements(outer_block));
+ ASSERT_TRUE(v.has_error());
+ // TODO(sarahM0): figure out what should be the error number.
+ EXPECT_EQ(v.error(),
+ "12:34 v-000x: invalid assignment: can't assign value of type "
+ "'__f32' to '__i32'");
+}
+
TEST_F(ValidatorTest, GlobalVariableWithStorageClass_Pass) {
// var<in> gloabl_var: f32;
AST().AddGlobalVariable(Var(Source{Source::Location{12, 34}}, "global_var",
@@ -502,6 +540,40 @@
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", ast::StorageClass::kNone, ty.f32(), Expr(2.0f),
+ ast::VariableDecorationList{});
+ 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,
+ });
+
+ EXPECT_TRUE(td()->DetermineStatements(outer_body)) << td()->error();
+ ASSERT_NE(TypeOf(lhs), nullptr);
+ ASSERT_NE(TypeOf(rhs), nullptr);
+
+ ValidatorImpl& v = Build();
+
+ 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;
@@ -688,6 +760,55 @@
EXPECT_EQ(v.error(), "12:34 v-0014: redeclared identifier 'a'");
}
+TEST_F(ValidatorTest, RedeclaredIdentifierInnerScopeBlock_Pass) {
+ // {
+ // { var a : f32; }
+ // var a : f32;
+ // }
+ auto* var_inner = Var("a", ast::StorageClass::kNone, ty.f32());
+ auto* inner = create<ast::BlockStatement>(ast::StatementList{
+ create<ast::VariableDeclStatement>(Source{Source::Location{12, 34}},
+ var_inner),
+ });
+
+ auto* var_outer = Var("a", ast::StorageClass::kNone, ty.f32());
+ auto* outer_body = create<ast::BlockStatement>(ast::StatementList{
+ inner,
+ create<ast::VariableDeclStatement>(var_outer),
+ });
+
+ EXPECT_TRUE(td()->DetermineStatements(outer_body)) << td()->error();
+
+ ValidatorImpl& v = Build();
+
+ EXPECT_TRUE(v.ValidateStatements(outer_body));
+}
+
+TEST_F(ValidatorTest, RedeclaredIdentifierInnerScopeBlock_Fail) {
+ // {
+ // var a : f32;
+ // { var a : f32; }
+ // }
+ auto* var_inner = Var("a", ast::StorageClass::kNone, ty.f32());
+ auto* inner = create<ast::BlockStatement>(ast::StatementList{
+ create<ast::VariableDeclStatement>(Source{Source::Location{12, 34}},
+ var_inner),
+ });
+
+ auto* var_outer = Var("a", ast::StorageClass::kNone, ty.f32());
+ auto* outer_body = create<ast::BlockStatement>(ast::StatementList{
+ create<ast::VariableDeclStatement>(var_outer),
+ inner,
+ });
+
+ EXPECT_TRUE(td()->DetermineStatements(outer_body)) << td()->error();
+
+ ValidatorImpl& v = Build();
+
+ EXPECT_FALSE(v.ValidateStatements(outer_body));
+ EXPECT_EQ(v.error(), "12:34 v-0014: redeclared identifier 'a'");
+}
+
TEST_F(ValidatorTest, RedeclaredIdentifierDifferentFunctions_Pass) {
// func0 { var a : f32 = 2.0; return; }
// func1 { var a : f32 = 3.0; return; }