resolver: Storable types include textures/samplers

Add a Resolver::IsPlain() method to check for plain types, which is
then used instead of IsStorable() for validating array and struct
subtypes.

Remove validation of assignment and constructor RHS types, instead
validating the type of the variable declaration. This catches
additional errors that were previously missed, such as using a pointer
for a var declaration with no constructor.

Change-Id: I5786a262159d2a42cc05b44743c6c26f6b5647c0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53960
Auto-Submit: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>

src/ast/module_clone_test.cc

@@ -66,7 +66,7 @@ fn f1(p0 : f32, p1 : i32) -> f32 {
   var l3 : vec2<u32> = vec2<u32>(u32(l0), u32(l1));
   var l4 : S;
   var l5 : u32 = l4.m1[5];
-  var l6 : ptr<private, u32>;
+  let l6 : ptr<private, u32> = &g0;
   loop {
     l0 = (p1 + 2);
     if (((l0 % 4) == 0)) {

src/resolver/assignment_validation_test.cc

@@ -155,7 +155,10 @@ TEST_F(ResolverAssignmentValidationTest, AssignToConstant_Fail) {
   EXPECT_EQ(r()->error(), "12:34 error: cannot assign to value of type 'i32'");
 }
 
-TEST_F(ResolverAssignmentValidationTest, AssignNonStorable_Fail) {
+// TODO(crbug.com/tint/809): The var has an implicit access::read, and so this
+// test will pass again when we start validating the access mode on the LHS of
+// an assignment.
+TEST_F(ResolverAssignmentValidationTest, DISABLED_AssignNonStorable_Fail) {
   // var a : texture_storage_1d<rgba8unorm, read>;
   // var b : texture_storage_1d<rgba8unorm, read>;
   // a = b;

src/resolver/resolver.cc

@@ -165,20 +165,18 @@ bool Resolver::Resolve() {
   return result;
 }
 
-// https://gpuweb.github.io/gpuweb/wgsl.html#storable-types
+// https://gpuweb.github.io/gpuweb/wgsl/#plain-types-section
-bool Resolver::IsStorable(const sem::Type* type) {
+bool Resolver::IsPlain(const sem::Type* type) {
-  if (type->is_scalar() || type->Is<sem::Vector>() || type->Is<sem::Matrix>()) {
+  if (type->is_scalar() || type->Is<sem::Vector>() || type->Is<sem::Matrix>() ||
+      type->Is<sem::Array>() || type->Is<sem::Struct>()) {
     return true;
   }
-  if (auto* arr = type->As<sem::Array>()) {
-    return IsStorable(arr->ElemType());
-  }
-  if (auto* str = type->As<sem::Struct>()) {
-    for (const auto* member : str->Members()) {
-      if (!IsStorable(member->Type())) {
   return false;
-      }
+}
-    }
+
+// https://gpuweb.github.io/gpuweb/wgsl.html#storable-types
+bool Resolver::IsStorable(const sem::Type* type) {
+  if (IsPlain(type) || type->Is<sem::Texture>() || type->Is<sem::Sampler>()) {
     return true;
   }
   return false;
@@ -525,14 +523,6 @@ bool Resolver::ValidateVariableConstructor(const ast::Variable* var,
                                            const std::string& rhs_type_name) {
   auto* value_type = rhs_type->UnwrapRef();  // Implicit load of RHS
 
-  // RHS needs to be of a storable type
-  if (!var->is_const() && !IsStorable(value_type)) {
-    diagnostics_.add_error(
-        "'" + rhs_type_name + "' is not storable for assignment",
-        var->constructor()->source());
-    return false;
-  }
-
   // Value type has to match storage type
   if (storage_type != value_type) {
     std::string decl = var->is_const() ? "let" : "var";
@@ -769,6 +759,14 @@ bool Resolver::ValidateGlobalVariable(const VariableInfo* info) {
 bool Resolver::ValidateVariable(const VariableInfo* info) {
   auto* var = info->declaration;
   auto* storage_type = info->type->UnwrapRef();
+
+  if (!var->is_const() && !IsStorable(storage_type)) {
+    diagnostics_.add_error(storage_type->FriendlyName(builder_->Symbols()) +
+                               " cannot be used as the type of a var",
+                           var->source());
+    return false;
+  }
+
   if (auto* r = storage_type->As<sem::Array>()) {
     if (r->IsRuntimeSized()) {
       diagnostics_.add_error(
@@ -2750,7 +2748,7 @@ sem::Array* Resolver::Array(const ast::Array* arr) {
     return nullptr;
   }
 
-  if (!IsStorable(el_ty)) {  // Check must come before DefaultAlignAndSize()
+  if (!IsPlain(el_ty)) {  // Check must come before DefaultAlignAndSize()
     builder_->Diagnostics().add_error(
         el_ty->FriendlyName(builder_->Symbols()) +
             " cannot be used as an element type of an array",
@@ -2924,7 +2922,7 @@ sem::Struct* Resolver::Structure(const ast::Struct* str) {
     }
 
     // Validate member type
-    if (!IsStorable(type)) {
+    if (!IsPlain(type)) {
       diagnostics_.add_error(
           type->FriendlyName(builder_->Symbols()) +
           " cannot be used as the type of a structure member");
@@ -3167,13 +3165,6 @@ bool Resolver::ValidateAssignment(const ast::AssignmentStatement* a) {
 
   auto* value_type = rhs_type->UnwrapRef();  // Implicit load of RHS
 
-  // RHS needs to be of a storable type
-  if (!IsStorable(value_type)) {
-    diagnostics_.add_error("'" + TypeNameOf(a->rhs()) + "' is not storable",
-                           a->rhs()->source());
-    return false;
-  }
-
   // Value type has to match storage type
   if (storage_type != value_type) {
     diagnostics_.add_error("cannot assign '" + TypeNameOf(a->rhs()) + "' to '" +

src/resolver/resolver.h

@@ -73,6 +73,10 @@ class Resolver {
   /// @returns true if the resolver was successful
   bool Resolve();
 
+  /// @param type the given type
+  /// @returns true if the given type is a plain type
+  bool IsPlain(const sem::Type* type);
+
   /// @param type the given type
   /// @returns true if the given type is storable
   bool IsStorable(const sem::Type* type);

src/resolver/var_let_validation_test.cc

@@ -43,18 +43,19 @@ TEST_F(ResolverVarLetValidationTest, GlobalLetNoInitializer) {
             "12:34 error: let declarations must have initializers");
 }
 
-TEST_F(ResolverVarLetValidationTest, VarConstructorNotStorable) {
+TEST_F(ResolverVarLetValidationTest, VarTypeNotStorable) {
   // var i : i32;
   // var p : pointer<function, i32> = &v;
   auto* i = Var("i", ty.i32(), ast::StorageClass::kNone);
-  auto* p = Var("a", ty.i32(), ast::StorageClass::kNone,
+  auto* p =
-                AddressOf(Source{{12, 34}}, "i"));
+      Var(Source{{56, 78}}, "a", ty.pointer<i32>(ast::StorageClass::kFunction),
+          ast::StorageClass::kNone, AddressOf(Source{{12, 34}}, "i"));
   WrapInFunction(i, p);
 
   EXPECT_FALSE(r()->Resolve());
   EXPECT_EQ(r()->error(),
-            "12:34 error: 'ptr<function, i32, read_write>' is not storable for "
+            "56:78 error: ptr<function, i32, read_write> cannot be used as the "
-            "assignment");
+            "type of a var");
 }
 
 TEST_F(ResolverVarLetValidationTest, LetConstructorWrongType) {