commit 4fb431c90eda5047b00d35e4577ce8e5ca554ee6
author Sarah Mashayekhi <sarahmashay@google.com> Tue Aug 25 15:04:53 2020 +0000
committer Commit Bot service account <commit-bot@chromium.org> Tue Aug 25 15:04:53 2020 +0000
tree 84110e1e5d1751920153def5d884ac5cf19e5630
parent deea295fc19a3ede4cf1cac66ad3368bcdae71f2

[validation] validates if global variables have a storage class

This CL moves the global variables checks to a function
Adds tests and checks for validation rule v-0022:
Global variables must have a storage class.

Bug: tint: 6
Change-Id: I2f2cd7df6e849bfd1ddfbca35568c6fc3345efa6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/27283
Commit-Queue: Sarah Mashayekhi <sarahmashay@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>

src/validator_impl.cc

diff --git a/src/validator_impl.cc b/src/validator_impl.cc
index 48fd7d2..968b2c6 100644
--- a/src/validator_impl.cc
+++ b/src/validator_impl.cc
@@ -35,13 +35,8 @@
     return false;
   }
   function_stack_.push_scope();
-  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 (!ValidateGlobalVariables(module->global_variables())) {
+    return false;
   }
   if (!CheckImports(module)) {
     return false;
@@ -59,6 +54,24 @@
   return true;
 }
 
+bool ValidatorImpl::ValidateGlobalVariables(
+    const ast::VariableList& global_vars) {
+  for (const auto& var : global_vars) {
+    if (variable_stack_.has(var->name())) {
+      set_error(var->source(),
+                "v-0011: redeclared global identifier '" + var->name() + "'");
+      return false;
+    }
+    if (var->storage_class() == ast::StorageClass::kNone) {
+      set_error(var->source(),
+                "v-0022: global variables must have a storage class");
+      return false;
+    }
+    variable_stack_.set_global(var->name(), var.get());
+  }
+  return true;
+}
+
 bool ValidatorImpl::ValidateEntryPoints(const ast::EntryPointList& eps) {
   for (const auto& ep : eps) {
     auto* ep_ptr = ep.get();

src/validator_impl.h

diff --git a/src/validator_impl.h b/src/validator_impl.h
index 3e38060..5f085fd 100644
--- a/src/validator_impl.h
+++ b/src/validator_impl.h
@@ -53,6 +53,10 @@
   /// @param src the source causing the error
   /// @param msg the error message
   void set_error(const Source& src, const std::string& msg);
+  /// Validate global variables
+  /// @param global_vars list of global variables to check
+  /// @returns true if the validation was successful
+  bool ValidateGlobalVariables(const ast::VariableList& global_vars);
   /// Validates Functions
   /// @param funcs the functions to check
   /// @returns true if the validation was successful

src/validator_test.cc

diff --git a/src/validator_test.cc b/src/validator_test.cc
index 3207ae7..6dc58c26 100644
--- a/src/validator_test.cc
+++ b/src/validator_test.cc
@@ -257,6 +257,30 @@
             "12:34: v-000x: invalid assignment of '__i32' to '__f32'");
 }
 
+TEST_F(ValidatorTest, GlobalVariableWithStorageClass_Pass) {
+  // var<in> gloabl_var: f32;
+  ast::type::F32Type f32;
+  auto global_var = std::make_unique<ast::Variable>(
+      Source{12, 34}, "global_var", ast::StorageClass::kInput, &f32);
+  mod()->AddGlobalVariable(std::move(global_var));
+  EXPECT_TRUE(td()->Determine()) << td()->error();
+  tint::ValidatorImpl v;
+  EXPECT_TRUE(v.Validate(mod())) << v.error();
+}
+
+TEST_F(ValidatorTest, GlobalVariableNoStorageClass_Fail) {
+  // var gloabl_var: f32;
+  ast::type::F32Type f32;
+  auto global_var = std::make_unique<ast::Variable>(
+      Source{12, 34}, "global_var", ast::StorageClass::kNone, &f32);
+  mod()->AddGlobalVariable(std::move(global_var));
+  EXPECT_TRUE(td()->Determine()) << td()->error();
+  tint::ValidatorImpl v;
+  EXPECT_FALSE(v.Validate(mod()));
+  EXPECT_EQ(v.error(),
+            "12:34: v-0022: global variables must have a storage class");
+}
+
 TEST_F(ValidatorTest, UsingUndefinedVariableGlobalVariable_Fail) {
   // var global_var: f32 = 2.1;
   // fn my_func() -> f32 {