validation: function scope variable store type must be constructible

- function scope variable store type must be constructible
- add IsConstructible() to sem::atomic

Bug: tint:1069
Change-Id: Ib0616b486ecf278dbdd99640dc4ede7f3007feb8
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/60120
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Sarah Mashayekhi <sarahmashay@google.com>
Auto-Submit: Sarah Mashayekhi <sarahmashay@google.com>

src/ast/module_clone_test.cc

@@ -27,12 +27,18 @@ TEST(ModuleCloneTest, Clone) {
   // Shader that exercises the bulk of the AST nodes and types.
   // See also fuzzers/tint_ast_clone_fuzzer.cc for further coverage of cloning.
   Source::File file("test.wgsl", R"([[block]]
-struct S {
+struct S0 {
   [[size(4)]]
   m0 : u32;
   m1 : array<u32>;
 };
 
+[[block]] struct S1 {
+  [[size(4)]]
+  m0 : u32;
+  m1 : array<u32, 6>;
+};
+
 let c0 : i32 = 10;
 let c1 : bool = true;
 
@@ -48,9 +54,9 @@ var<private> g1 : f32 = 123.0;
 [[group(4), binding(0)]] var g6 : texture_storage_2d<rg32float, write>;
 
 var<private> g7 : vec3<f32>;
-[[group(0), binding(1)]] var<storage, write> g8 : S;
+[[group(0), binding(1)]] var<storage, write> g8 : S0;
-[[group(1), binding(1)]] var<storage, read> g9 : S;
+[[group(1), binding(1)]] var<storage, read> g9 : S0;
-[[group(2), binding(1)]] var<storage, read_write> g10 : S;
+[[group(2), binding(1)]] var<storage, read_write> g10 : S0;
 
 fn f0(p0 : bool) -> f32 {
   if (p0) {
@@ -64,7 +70,7 @@ fn f1(p0 : f32, p1 : i32) -> f32 {
   var l1 : f32 = 8.0;
   var l2 : u32 = bitcast<u32>(4);
   var l3 : vec2<u32> = vec2<u32>(u32(l0), u32(l1));
-  var l4 : S;
+  var l4 : S1;
   var l5 : u32 = l4.m1[5];
   let l6 : ptr<private, u32> = &g0;
   loop {

src/resolver/atomics_validation_test.cc

@@ -34,7 +34,8 @@ TEST_F(ResolverAtomicValidationTest, GlobalOfInvalidType) {
   EXPECT_EQ(r()->error(), "12:34 error: atomic only supports i32 or u32 types");
 }
 
-TEST_F(ResolverAtomicValidationTest, GlobalOfInvalidStorageClass) {
+// TODO(crbug.com/tint/909): add validation and enable this test
+TEST_F(ResolverAtomicValidationTest, DISABLED_GlobalOfInvalidStorageClass) {
   Global("a", ty.atomic(Source{{12, 34}}, ty.i32()),
          ast::StorageClass::kPrivate);
 
@@ -60,7 +61,8 @@ TEST_F(ResolverAtomicValidationTest, Local) {
   WrapInFunction(Var("a", ty.atomic(Source{{12, 34}}, ty.i32())));
 
   EXPECT_FALSE(r()->Resolve());
-  EXPECT_EQ(r()->error(), "12:34 error: atomic var requires workgroup storage");
+  EXPECT_EQ(r()->error(),
+            "12:34 error: function variable must have a constructible type");
 }
 
 TEST_F(ResolverAtomicValidationTest, NoAtomicExpr) {

src/resolver/resolver.cc

@@ -1122,19 +1122,6 @@ bool Resolver::ValidateVariable(const VariableInfo* info) {
     AddError("invalid use of input/output storage class", var->source());
     return false;
   }
-
-  // https://gpuweb.github.io/gpuweb/wgsl/#atomic-types
-  // Atomic types may only be instantiated by variables in the workgroup storage
-  // class or by storage buffer variables with a read_write access mode.
-  if (info->type->UnwrapRef()->Is<sem::Atomic>() &&
-      info->storage_class != ast::StorageClass::kWorkgroup) {
-    // Storage buffers require a structure, so just check for workgroup
-    // storage here.
-    AddError("atomic var requires workgroup storage",
-             info->declaration->type()->source());
-    return false;
-  }
-
   return true;
 }
 
@@ -3370,10 +3357,15 @@ bool Resolver::VariableDeclStatement(const ast::VariableDeclStatement* stmt) {
     return false;
   }
 
-  if (!var->is_const()) {
+  if (!var->is_const() &&
-    if (info->storage_class != ast::StorageClass::kFunction &&
+      IsValidationEnabled(var->decorations(),
-        IsValidationEnabled(var->decorations(),
+                          ast::DisabledValidation::kIgnoreStorageClass)) {
-                            ast::DisabledValidation::kIgnoreStorageClass)) {
+    if (!info->type->UnwrapRef()->IsConstructible()) {
+      AddError("function variable must have a constructible type",
+               var->type()->source());
+      return false;
+    }
+    if (info->storage_class != ast::StorageClass::kFunction) {
       if (info->storage_class != ast::StorageClass::kNone) {
         AddError("function variable has a non-function storage class",
                  stmt->source());

src/resolver/validation_test.cc

@@ -352,8 +352,8 @@ TEST_F(ResolverValidationTest, StorageClass_FunctionVariableWorkgroupClass) {
             "error: function variable has a non-function storage class");
 }
 
-TEST_F(ResolverValidationTest, StorageClass_FunctionVariableHandleClass) {
+TEST_F(ResolverValidationTest, StorageClass_FunctionVariableI32) {
-  auto* var = Var("s", ty.sampler(ast::SamplerKind::kSampler));
+  auto* var = Var("s", ty.i32(), ast::StorageClass::kPrivate);
 
   auto* stmt = Decl(var);
   Func("func", ast::VariableList{}, ty.void_(), ast::StatementList{stmt},

src/resolver/var_let_validation_test.cc

@@ -298,6 +298,35 @@ TEST_F(ResolverVarLetValidationTest, InferredPtrStorageAccessMismatch) {
             "'ptr<storage, i32, read>'");
 }
 
+TEST_F(ResolverVarLetValidationTest, NonConstructibleType_Atomic) {
+  auto* v = Var("v", ty.atomic(Source{{12, 34}}, ty.i32()));
+  WrapInFunction(v);
+
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(r()->error(),
+            "12:34 error: function variable must have a constructible type");
+}
+
+TEST_F(ResolverVarLetValidationTest, NonConstructibleType_RuntimeArray) {
+  auto* s = Structure("S", {Member("m", ty.array(ty.i32()))}, {StructBlock()});
+  auto* v = Var("v", ty.Of(s));
+  WrapInFunction(v);
+
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(r()->error(),
+            "error: function variable must have a constructible type");
+}
+
+TEST_F(ResolverVarLetValidationTest, NonConstructibleType_Struct_WithAtomic) {
+  auto* s = Structure("S", {Member("m", ty.atomic(ty.i32()))});
+  auto* v = Var("v", ty.Of(s));
+  WrapInFunction(v);
+
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(r()->error(),
+            "error: function variable must have a constructible type");
+}
+
 }  // namespace
 }  // namespace resolver
 }  // namespace tint

src/sem/atomic_type.cc

@@ -46,6 +46,10 @@ uint32_t Atomic::Align() const {
   return subtype_->Align();
 }
 
+bool Atomic::IsConstructible() const {
+  return false;
+}
+
 Atomic::Atomic(Atomic&&) = default;
 
 Atomic::~Atomic() = default;

src/sem/atomic_type.h

@@ -50,6 +50,10 @@ class Atomic : public Castable<Atomic, Type> {
   /// @returns the alignment in bytes of the type.
   uint32_t Align() const override;
 
+  /// @returns true if constructible as per
+  /// https://gpuweb.github.io/gpuweb/wgsl/#constructible-typesd
+  bool IsConstructible() const override;
+
  private:
   sem::Type const* const subtype_;
 };