commit cf28b5aafe8d6b5c2f03b0c1d31b86d208ecb133
author Ben Clayton <bclayton@google.com> Mon Apr 19 20:23:12 2021 +0000
committer Commit Bot service account <commit-bot@chromium.org> Mon Apr 19 20:23:12 2021 +0000
tree b5a8622309f9fd86f0851d88c0df5b5db2242031
parent 85bfea6edd0d3259bc7dd65d6d61e5c5fbf85276

resolver: Call ValidateVariable() for globals

This was only called for function-scope variable declarations.
In calling this, there were inevitable tests failing, which have now been fixed.
Added a test for the single runtime-array-length validation rule that this function was checking.

Fixed: tint:345
Change-Id: Ic453c38158c1290a5e1ef6de56af0c621d97982a
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48381
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: David Neto <dneto@google.com>

src/resolver/intrinsic_test.cc

diff --git a/src/resolver/intrinsic_test.cc b/src/resolver/intrinsic_test.cc
index 9d67ef1..60461fb 100644
--- a/src/resolver/intrinsic_test.cc
+++ b/src/resolver/intrinsic_test.cc
@@ -25,6 +25,7 @@
 #include "src/ast/loop_statement.h"
 #include "src/ast/return_statement.h"
 #include "src/ast/stage_decoration.h"
+#include "src/ast/struct_block_decoration.h"
 #include "src/ast/switch_statement.h"
 #include "src/ast/unary_op_expression.h"
 #include "src/ast/variable_decl_statement.h"
@@ -758,8 +759,13 @@
 using ResolverIntrinsicDataTest = ResolverTest;
 
 TEST_F(ResolverIntrinsicDataTest, ArrayLength_Vector) {
-  Global("arr", ty.array<int>(), ast::StorageClass::kInput);
-  auto* call = Call("arrayLength", "arr");
+  auto* ary = ty.array<i32>();
+  auto* str = Structure("S", {Member("x", ary)},
+                        {create<ast::StructBlockDecoration>()});
+  auto* ac = ty.access(ast::AccessControl::kReadOnly, str);
+  Global("a", ac, ast::StorageClass::kStorage);
+
+  auto* call = Call("arrayLength", MemberAccessor("a", "x"));
   WrapInFunction(call);
 
   EXPECT_TRUE(r()->Resolve()) << r()->error();

src/resolver/resolver.cc

diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index 783e997..31f9dcc 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -353,7 +353,7 @@
     }
   }
 
-  return true;
+  return ValidateVariable(info->declaration);
 }
 
 bool Resolver::ValidateVariable(const ast::Variable* var) {

src/resolver/type_validation_test.cc

diff --git a/src/resolver/type_validation_test.cc b/src/resolver/type_validation_test.cc
index e580129..138efe9 100644
--- a/src/resolver/type_validation_test.cc
+++ b/src/resolver/type_validation_test.cc
@@ -371,9 +371,31 @@
   WrapInFunction();
 
   EXPECT_FALSE(r()->Resolve()) << r()->error();
-  EXPECT_EQ(r()->error(),
-            "12:34 error v-0015: runtime arrays may only appear as the last "
-            "member of a struct");
+  EXPECT_EQ(
+      r()->error(),
+      R"(12:34 error v-0015: runtime arrays may only appear as the last member of a struct)");
+}
+
+TEST_F(ResolverTypeValidationTest, RuntimeArrayAsGlobalVariable) {
+  Global(Source{{56, 78}}, "g", ty.array<i32>(), ast::StorageClass::kPrivate);
+
+  ASSERT_FALSE(r()->Resolve());
+
+  EXPECT_EQ(
+      r()->error(),
+      R"(56:78 error v-0015: runtime arrays may only appear as the last member of a struct)");
+}
+
+TEST_F(ResolverTypeValidationTest, RuntimeArrayAsLocalVariable) {
+  auto* v =
+      Var(Source{{56, 78}}, "g", ty.array<i32>(), ast::StorageClass::kFunction);
+  WrapInFunction(v);
+
+  ASSERT_FALSE(r()->Resolve());
+
+  EXPECT_EQ(
+      r()->error(),
+      R"(56:78 error v-0015: runtime arrays may only appear as the last member of a struct)");
 }
 
 TEST_F(ResolverTypeValidationTest, RuntimeArrayAsParameter_Fail) {

src/writer/spirv/builder_type_test.cc

diff --git a/src/writer/spirv/builder_type_test.cc b/src/writer/spirv/builder_type_test.cc
index fbd1645..964d227 100644
--- a/src/writer/spirv/builder_type_test.cc
+++ b/src/writer/spirv/builder_type_test.cc
@@ -59,7 +59,10 @@
 
 TEST_F(BuilderTest_Type, GenerateRuntimeArray) {
   auto* ary = ty.array(ty.i32(), 0);
-  Global("a", ary, ast::StorageClass::kInput);
+  auto* str = Structure("S", {Member("x", ary)},
+                        {create<ast::StructBlockDecoration>()});
+  auto* ac = ty.access(ast::AccessControl::kReadOnly, str);
+  Global("a", ac, ast::StorageClass::kStorage);
 
   spirv::Builder& b = Build();
 
@@ -74,7 +77,10 @@
 
 TEST_F(BuilderTest_Type, ReturnsGeneratedRuntimeArray) {
   auto* ary = ty.array(ty.i32(), 0);
-  Global("a", ary, ast::StorageClass::kInput);
+  auto* str = Structure("S", {Member("x", ary)},
+                        {create<ast::StructBlockDecoration>()});
+  auto* ac = ty.access(ast::AccessControl::kReadOnly, str);
+  Global("a", ac, ast::StorageClass::kStorage);
 
   spirv::Builder& b = Build();