[validation] impl v-0015: runtime array may only appear last

This CL adds checks to verify that runtime arrays only appear as the last element of a struct

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

src/validator/validator_impl.cc

@@ -22,8 +22,11 @@
 #include "src/ast/int_literal.h"
 #include "src/ast/intrinsic.h"
 #include "src/ast/sint_literal.h"
+#include "src/ast/struct.h"
 #include "src/ast/switch_statement.h"
+#include "src/ast/type/array_type.h"
 #include "src/ast/type/i32_type.h"
+#include "src/ast/type/struct_type.h"
 #include "src/ast/type/u32_type.h"
 #include "src/ast/type/void_type.h"
 #include "src/ast/uint_literal.h"
@@ -48,6 +51,9 @@ bool ValidatorImpl::Validate(const ast::Module* module) {
   if (!ValidateGlobalVariables(module->global_variables())) {
     return false;
   }
+  if (!ValidateConstructedTypes(module->constructed_types())) {
+    return false;
+  }
   if (!ValidateFunctions(module->functions())) {
     return false;
   }
@@ -59,6 +65,28 @@ bool ValidatorImpl::Validate(const ast::Module* module) {
   return true;
 }
 
+bool ValidatorImpl::ValidateConstructedTypes(
+    const std::vector<ast::type::Type*>& constructed_types) {
+  for (auto* const ct : constructed_types) {
+    if (ct->IsStruct()) {
+      auto* st = ct->AsStruct();
+      for (auto* member : st->impl()->members()) {
+        if (member->type()->UnwrapAll()->IsArray()) {
+          auto* r = member->type()->UnwrapAll()->AsArray();
+          if (r->IsRuntimeArray() && member != st->impl()->members().back()) {
+            set_error(member->source(),
+                      "v-0015: runtime arrays may only appear as the last "
+                      "member of a struct: '" +
+                          member->name() + "'");
+            return false;
+          }
+        }
+      }
+    }
+  }
+  return true;
+}
+
 bool ValidatorImpl::ValidateGlobalVariables(
     const ast::VariableList& global_vars) {
   for (auto* var : global_vars) {

src/validator/validator_impl.h

@@ -17,6 +17,7 @@
 
 #include <string>
 #include <unordered_map>
+#include <vector>
 
 #include "src/ast/assignment_statement.h"
 #include "src/ast/call_expression.h"
@@ -118,6 +119,12 @@ class ValidatorImpl {
   /// @returns true if the valdiation was successful
   bool ValidateEntryPoint(const ast::FunctionList& funcs);
 
+  /// Validates constructed types
+  /// @param constructed_types the types to check
+  /// @returns true if the valdiation was successful
+  bool ValidateConstructedTypes(
+      const std::vector<ast::type::Type*>& constructed_types);
+
  private:
   std::string error_;
   ScopeStack<ast::Variable*> variable_stack_;

src/validator/validator_type_test.cc

@@ -17,6 +17,7 @@
 #include "src/ast/struct.h"
 #include "src/ast/struct_member.h"
 #include "src/ast/struct_member_decoration.h"
+#include "src/ast/type/alias_type.h"
 #include "src/ast/type/array_type.h"
 #include "src/ast/type/f32_type.h"
 #include "src/ast/type/struct_type.h"
@@ -51,11 +52,11 @@ TEST_F(ValidatorTypeTest, RuntimeArrayIsLast_Pass) {
   auto* st = create<ast::Struct>(decos, members);
   ast::type::StructType struct_type("Foo", st);
 
-  // mod()->AddConstructedType(&struct_type);
+  mod()->AddConstructedType(&struct_type);
-  // EXPECT_TRUE(v()->ValidateConstructedTypes(mod()->constructed_types()));
+  EXPECT_TRUE(v()->ValidateConstructedTypes(mod()->constructed_types()));
 }
 
-TEST_F(ValidatorTypeTest, DISABLED_RuntimeArrayIsNotLast_Fail) {
+TEST_F(ValidatorTypeTest, RuntimeArrayIsNotLast_Fail) {
   // struct Foo {
   //   rt: array<f32>;
   //   vf: f32;
@@ -78,10 +79,71 @@ TEST_F(ValidatorTypeTest, DISABLED_RuntimeArrayIsNotLast_Fail) {
   ast::type::StructType struct_type("Foo", st);
 
   mod()->AddConstructedType(&struct_type);
-  // EXPECT_FALSE(v()->ValidateConstructedTypes(mod()->constructed_types()));
+  EXPECT_FALSE(v()->ValidateConstructedTypes(mod()->constructed_types()));
-  // EXPECT_EQ(v()->error(),
+  EXPECT_EQ(v()->error(),
-  //           "12:34: v-0015: runtime arrays may only appear as the last member
+            "12:34: v-0015: runtime arrays may only appear as the last member "
-  //           " "of a struct: 'rt'");
+            "of a struct: 'rt'");
 }
+
+TEST_F(ValidatorTypeTest, AliasRuntimeArrayIsNotLast_Fail) {
+  // type RTArr = array<u32>;
+  // struct s {
+  //  b: RTArr;
+  //  a: u32;
+  //}
+
+  ast::type::F32Type u32;
+  ast::type::ArrayType array(&u32);
+  ast::type::AliasType alias{"RTArr", &array};
+
+  ast::StructMemberList members;
+  {
+    ast::StructMemberDecorationList deco;
+    members.push_back(create<ast::StructMember>(
+        Source{Source::Location{12, 34}}, "b", &alias, deco));
+  }
+  {
+    ast::StructMemberDecorationList deco;
+    members.push_back(create<ast::StructMember>("a", &u32, deco));
+  }
+
+  ast::StructDecorationList decos;
+  auto* st = create<ast::Struct>(decos, members);
+  ast::type::StructType struct_type("s", st);
+  mod()->AddConstructedType(&struct_type);
+  EXPECT_FALSE(v()->ValidateConstructedTypes(mod()->constructed_types()));
+  EXPECT_EQ(v()->error(),
+            "12:34: v-0015: runtime arrays may only appear as the last member "
+            "of a struct: 'b'");
+}
+
+TEST_F(ValidatorTypeTest, AliasRuntimeArrayIsLast_Pass) {
+  // type RTArr = array<u32>;
+  // struct s {
+  //  a: u32;
+  //  b: RTArr;
+  //}
+
+  ast::type::F32Type u32;
+  ast::type::ArrayType array(&u32);
+  ast::type::AliasType alias{"RTArr", &array};
+
+  ast::StructMemberList members;
+  {
+    ast::StructMemberDecorationList deco;
+    members.push_back(create<ast::StructMember>("a", &u32, deco));
+  }
+  {
+    ast::StructMemberDecorationList deco;
+    members.push_back(create<ast::StructMember>(
+        Source{Source::Location{12, 34}}, "b", &alias, deco));
+  }
+  ast::StructDecorationList decos;
+  auto* st = create<ast::Struct>(decos, members);
+  ast::type::StructType struct_type("s", st);
+  mod()->AddConstructedType(&struct_type);
+  EXPECT_TRUE(v()->ValidateConstructedTypes(mod()->constructed_types()));
+}
+
 }  // namespace
 }  // namespace tint