validation: cannot store into a read-only type

Bug: tint:443
Change-Id: I4ebb14ab53b2f72d108efb4770241c687bf86ee2
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54484
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Sarah Mashayekhi <sarahmashay@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>

src/resolver/assignment_validation_test.cc

@@ -15,6 +15,7 @@
 #include "src/resolver/resolver.h"
 
 #include "gmock/gmock.h"
+#include "src/ast/struct_block_decoration.h"
 #include "src/resolver/resolver_test_helper.h"
 #include "src/sem/storage_texture_type.h"
 
@@ -24,6 +25,27 @@ namespace {
 
 using ResolverAssignmentValidationTest = ResolverTest;
 
+TEST_F(ResolverAssignmentValidationTest, ReadOnlyBuffer) {
+  // [[block]] struct S { m : i32 };
+  // [[group(0), binding(0)]]
+  // var<storage,read> a : S;
+  auto* s = Structure("S", {Member("m", ty.i32())},
+                      {create<ast::StructBlockDecoration>()});
+  Global(Source{{12, 34}}, "a", ty.Of(s), ast::StorageClass::kStorage,
+         ast::Access::kRead,
+         ast::DecorationList{
+             create<ast::BindingDecoration>(0),
+             create<ast::GroupDecoration>(0),
+         });
+
+  WrapInFunction(Assign(Source{{56, 78}}, MemberAccessor("a", "m"), 1));
+
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(r()->error(),
+            "56:78 error: cannot store into a read-only type 'ref<storage, "
+            "i32, read>'");
+}
+
 TEST_F(ResolverAssignmentValidationTest, AssignIncompatibleTypes) {
   // {
   //  var a : i32 = 2;
@@ -154,10 +176,7 @@ TEST_F(ResolverAssignmentValidationTest, AssignToConstant_Fail) {
   EXPECT_EQ(r()->error(), "12:34 error: cannot assign to value of type 'i32'");
 }
 
-// TODO(crbug.com/tint/809): The var has an implicit access::read, and so this
+TEST_F(ResolverAssignmentValidationTest, AssignNonStorable_Fail) {
-// 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;
@@ -179,12 +198,12 @@ TEST_F(ResolverAssignmentValidationTest, DISABLED_AssignNonStorable_Fail) {
              create<ast::GroupDecoration>(0),
          });
 
-  WrapInFunction(Assign("a", Expr(Source{{12, 34}}, "b")));
+  WrapInFunction(Assign(Source{{56, 78}}, "a", "b"));
 
   EXPECT_FALSE(r()->Resolve());
-  EXPECT_EQ(
+  EXPECT_EQ(r()->error(),
-      r()->error(),
+            "56:78 error: cannot store into a read-only type "
-      R"(12:34 error: 'texture_storage_1d<rgba8unorm, read>' is not storable)");
+            "'texture_storage_1d<rgba8unorm, read>'");
 }
 
 }  // namespace

src/resolver/resolver.cc

@@ -3317,11 +3317,6 @@ bool Resolver::ValidateAssignment(const ast::AssignmentStatement* a) {
   }
 
   auto* storage_type = lhs_ref->StoreType();
-
-  // TODO(crbug.com/tint/809): The originating variable of the left-hand side
-  // must not have an access(read) access attribute.
-  // https://gpuweb.github.io/gpuweb/wgsl/#assignment
-
   auto* value_type = rhs_type->UnwrapRef();  // Implicit load of RHS
 
   // Value type has to match storage type
@@ -3331,7 +3326,12 @@ bool Resolver::ValidateAssignment(const ast::AssignmentStatement* a) {
                            a->source());
     return false;
   }
-
+  if (lhs_ref->Access() == ast::Access::kRead) {
+    diagnostics_.add_error(
+        "cannot store into a read-only type '" + TypeNameOf(a->lhs()) + "'",
+        a->source());
+    return false;
+  }
   return true;
 }
 

src/resolver/resolver_test.cc

@@ -814,7 +814,7 @@ TEST_F(ResolverTest, Function_RegisterInputOutputVariables) {
   auto* in_var = Global("in_var", ty.f32(), ast::StorageClass::kInput);
   auto* out_var = Global("out_var", ty.f32(), ast::StorageClass::kOutput);
   auto* sb_var = Global("sb_var", ty.Of(s), ast::StorageClass::kStorage,
-                        ast::Access::kRead,
+                        ast::Access::kReadWrite,
                         ast::DecorationList{
                             create<ast::BindingDecoration>(0),
                             create<ast::GroupDecoration>(0),
@@ -853,7 +853,7 @@ TEST_F(ResolverTest, Function_RegisterInputOutputVariables_SubFunction) {
   auto* in_var = Global("in_var", ty.f32(), ast::StorageClass::kInput);
   auto* out_var = Global("out_var", ty.f32(), ast::StorageClass::kOutput);
   auto* sb_var = Global("sb_var", ty.Of(s), ast::StorageClass::kStorage,
-                        ast::Access::kRead,
+                        ast::Access::kReadWrite,
                         ast::DecorationList{
                             create<ast::BindingDecoration>(0),
                             create<ast::GroupDecoration>(0),