[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>

src/validator/validator_impl.cc

@@ -481,12 +481,18 @@ bool ValidatorImpl::ValidateStatements(const ast::BlockStatement* block) {
   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 @@ bool ValidatorImpl::ValidateStatement(const ast::Statement* stmt) {
   if (auto* c = stmt->As<ast::CaseStatement>()) {
     return ValidateCase(c);
   }
+  if (auto* b = stmt->As<ast::BlockStatement>()) {
+    return ValidateStatements(b);
+  }
   return true;
 }
 

src/validator/validator_test.cc

@@ -326,6 +326,44 @@ TEST_F(ValidatorTest, AssignIncompatibleTypesInBlockStatement_Fail) {
             "'__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 @@ TEST_F(ValidatorTest, UsingUndefinedVariableOuterScope_Pass) {
   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 @@ TEST_F(ValidatorTest, DISABLED_RedeclaredIdentifierInnerScope_False) {
   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; }