Remove ast::AccessControl::kInvalid

Also spruce up texture validation tests so that they validate error
messages.

Bug: tint:805
Change-Id: I6c86fc16014b127a7ef8254e5badf9b5bed08623
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51860
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
diff --git a/src/ast/access_control.cc b/src/ast/access_control.cc
index 52e3532..635eeb6 100644
--- a/src/ast/access_control.cc
+++ b/src/ast/access_control.cc
@@ -37,9 +37,6 @@
 std::string AccessControl::type_name() const {
   std::string name = "__access_control_";
   switch (access_) {
-    case ast::AccessControl::kInvalid:
-      name += "invalid";
-      break;
     case ast::AccessControl::kReadOnly:
       name += "read_only";
       break;
@@ -57,9 +54,6 @@
   std::ostringstream out;
   out << "[[access(";
   switch (access_) {
-    case ast::AccessControl::kInvalid:
-      out << "invalid";
-      break;
     case ast::AccessControl::kReadOnly:
       out << "read";
       break;
@@ -83,10 +77,6 @@
 
 std::ostream& operator<<(std::ostream& out, AccessControl::Access access) {
   switch (access) {
-    case ast::AccessControl::kInvalid: {
-      out << "invalid";
-      break;
-    }
     case ast::AccessControl::kReadOnly: {
       out << "read_only";
       break;
diff --git a/src/ast/access_control.h b/src/ast/access_control.h
index 23ce43d..fa5f92a 100644
--- a/src/ast/access_control.h
+++ b/src/ast/access_control.h
@@ -28,9 +28,6 @@
  public:
   /// The access control settings
   enum Access {
-    /// Invalid
-    // TODO(crbug.com/tint/805): Remove this.
-    kInvalid = -1,
     /// Read only
     kReadOnly,
     /// Write only
diff --git a/src/inspector/inspector.cc b/src/inspector/inspector.cc
index b687b22..527e0d1 100644
--- a/src/inspector/inspector.cc
+++ b/src/inspector/inspector.cc
@@ -641,10 +641,6 @@
     auto* var = rsv.first;
     auto binding_info = rsv.second;
 
-    if (var->AccessControl() == ast::AccessControl::kInvalid) {
-      continue;
-    }
-
     if (read_only != (var->AccessControl() == ast::AccessControl::kReadOnly)) {
       continue;
     }
diff --git a/src/intrinsic_table.cc b/src/intrinsic_table.cc
index e313d0a..bcbb5c3 100644
--- a/src/intrinsic_table.cc
+++ b/src/intrinsic_table.cc
@@ -40,7 +40,8 @@
 enum class OpenNumber {
   N,  // Typically used for vecN
   M,  // Typically used for matNxM
-  F,  // Typically used for texture_storage_2d<F>
+  F,  // Typically used F in for texture_storage_2d<F, A>
+  A,  // Typically used for A in texture_storage_2d<F, A>
 };
 
 /// @return a string of the OpenType symbol `ty`
@@ -64,6 +65,8 @@
       return "M";
     case OpenNumber::F:
       return "F";
+    case OpenNumber::A:
+      return "A";
   }
   return "";
 }
@@ -516,13 +519,23 @@
 /// the given texel and channel formats.
 class StorageTextureBuilder : public Builder {
  public:
-  explicit StorageTextureBuilder(
-      ast::TextureDimension dimensions,
-      ast::AccessControl::Access access,
-      OpenNumber texel_format,  // a.k.a "image format"
-      OpenType channel_format)  // a.k.a "storage subtype"
+  StorageTextureBuilder(ast::TextureDimension dimensions,
+                        ast::AccessControl::Access access,
+                        OpenNumber texel_format,  // a.k.a "image format"
+                        OpenType channel_format)  // a.k.a "storage subtype"
       : dimensions_(dimensions),
         access_(access),
+        access_is_open_num_(false),
+        texel_format_(texel_format),
+        channel_format_(channel_format) {}
+
+  StorageTextureBuilder(ast::TextureDimension dimensions,
+                        OpenNumber access,
+                        OpenNumber texel_format,  // a.k.a "image format"
+                        OpenType channel_format)  // a.k.a "storage subtype"
+      : dimensions_(dimensions),
+        access_(access),
+        access_is_open_num_(true),
         texel_format_(texel_format),
         channel_format_(channel_format) {}
 
@@ -531,10 +544,16 @@
       if (MatchOpenNumber(state, texel_format_,
                           static_cast<uint32_t>(tex->image_format()))) {
         if (MatchOpenType(state, channel_format_, tex->type())) {
-          // AccessControl::kInvalid means match any
-          if (access_ != ast::AccessControl::kInvalid &&
-              access_ != tex->access_control()) {
-            return false;
+          if (access_is_open_num_) {
+            if (!MatchOpenNumber(
+                    state, access_.open_num,
+                    static_cast<uint32_t>(tex->access_control()))) {
+              return false;
+            }
+          } else {
+            if (access_.enum_val != tex->access_control()) {
+              return false;
+            }
           }
 
           return tex->dim() == dimensions_;
@@ -547,9 +566,13 @@
   sem::Type* Build(BuildState& state) const override {
     auto texel_format =
         static_cast<ast::ImageFormat>(state.open_numbers.at(texel_format_));
+    auto access = access_is_open_num_
+                      ? static_cast<ast::AccessControl::Access>(
+                            state.open_numbers.at(access_.open_num))
+                      : access_.enum_val;
     auto* channel_format = state.open_types.at(channel_format_);
     return state.ty_mgr.Get<sem::StorageTexture>(
-        dimensions_, texel_format, access_,
+        dimensions_, texel_format, access,
         const_cast<sem::Type*>(channel_format));
   }
 
@@ -557,10 +580,10 @@
     std::stringstream ss;
 
     ss << "texture_storage_" << dimensions_ << "<F, ";
-    if (access_ == ast::AccessControl::Access::kInvalid) {
+    if (access_is_open_num_) {
       ss << "A";
     } else {
-      ss << access_;
+      ss << access_.enum_val;
     }
     ss << ">";
 
@@ -569,7 +592,14 @@
 
  private:
   ast::TextureDimension const dimensions_;
-  ast::AccessControl::Access const access_;
+  union Access {
+    Access(OpenNumber in) : open_num(in) {}
+    Access(ast::AccessControl::Access in) : enum_val(in) {}
+
+    OpenNumber const open_num;
+    ast::AccessControl::Access const enum_val;
+  } access_;
+  bool access_is_open_num_;
   OpenNumber const texel_format_;
   OpenType const channel_format_;
 };
@@ -748,6 +778,14 @@
         dimensions, access, texel_format, channel_format);
   }
 
+  Builder* storage_texture(ast::TextureDimension dimensions,
+                           OpenNumber access,
+                           OpenNumber texel_format,
+                           OpenType channel_format) {
+    return matcher_allocator_.Create<StorageTextureBuilder>(
+        dimensions, access, texel_format, channel_format);
+  }
+
   /// @returns a Matcher / Builder that matches an external texture
   Builder* external_texture() {
     return matcher_allocator_.Create<ExternalTextureBuilder>();
@@ -1079,14 +1117,14 @@
   auto* tex_depth_cube = depth_texture(Dim::kCube);
   auto* tex_depth_cube_array = depth_texture(Dim::kCubeArray);
   auto* tex_external = external_texture();
-  auto* tex_storage_1d_FT = storage_texture(
-      Dim::k1d, ast::AccessControl::kInvalid, OpenNumber::F, OpenType::T);
-  auto* tex_storage_2d_FT = storage_texture(
-      Dim::k2d, ast::AccessControl::kInvalid, OpenNumber::F, OpenType::T);
-  auto* tex_storage_2d_array_FT = storage_texture(
-      Dim::k2dArray, ast::AccessControl::kInvalid, OpenNumber::F, OpenType::T);
-  auto* tex_storage_3d_FT = storage_texture(
-      Dim::k3d, ast::AccessControl::kInvalid, OpenNumber::F, OpenType::T);
+  auto* tex_storage_1d_FT =
+      storage_texture(Dim::k1d, OpenNumber::A, OpenNumber::F, OpenType::T);
+  auto* tex_storage_2d_FT =
+      storage_texture(Dim::k2d, OpenNumber::A, OpenNumber::F, OpenType::T);
+  auto* tex_storage_2d_array_FT =
+      storage_texture(Dim::k2dArray, OpenNumber::A, OpenNumber::F, OpenType::T);
+  auto* tex_storage_3d_FT =
+      storage_texture(Dim::k3d, OpenNumber::A, OpenNumber::F, OpenType::T);
   auto* tex_storage_ro_1d_FT = storage_texture(
       Dim::k1d, ast::AccessControl::kReadOnly, OpenNumber::F, OpenType::T);
   auto* tex_storage_ro_2d_FT = storage_texture(
diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index 384cfde..e63800f 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -284,7 +284,7 @@
       return builder_->create<sem::F32>();
     }
     if (auto* t = ty->As<ast::AccessControl>()) {
-      TINT_SCOPED_ASSIGNMENT(curent_access_control_, t);
+      TINT_SCOPED_ASSIGNMENT(current_access_control_, t);
       if (auto* el = Type(t->type())) {
         return el;
       }
@@ -337,18 +337,15 @@
     }
     if (auto* t = ty->As<ast::StorageTexture>()) {
       if (auto* el = Type(t->type())) {
-        auto ac = ast::AccessControl::kInvalid;
-        if (curent_access_control_) {
-          ac = curent_access_control_->access_control();
-        } else {
-          // TODO(amaiorano): move error about missing access control on storage
-          // textures here, instead of when variables declared. That way, we'd
-          // get the error on the alias line (for
-          // alias<accesscontrol<storagetexture>>).
+        if (!current_access_control_) {
+          diagnostics_.add_error("storage textures must have access control",
+                                 t->source());
+          return nullptr;
         }
-
         return builder_->create<sem::StorageTexture>(
-            t->dim(), t->image_format(), ac, const_cast<sem::Type*>(el));
+            t->dim(), t->image_format(),
+            current_access_control_->access_control(),
+            const_cast<sem::Type*>(el));
       }
       return nullptr;
     }
@@ -485,11 +482,7 @@
     return nullptr;
   };
 
-  auto access_control = ast::AccessControl::kInvalid;
-  if (auto* ac = find_first_access_control(var->type())) {
-    access_control = ac->access_control();
-  }
-
+  auto* access_control = find_first_access_control(var->type());
   auto* info = variable_infos_.Create(var, const_cast<sem::Type*>(type),
                                       type_name, storage_class, access_control);
   variable_to_info_.emplace(var, info);
@@ -666,7 +659,7 @@
       // Its store type must be a host-shareable structure type with block
       // attribute, satisfying the storage class constraints.
 
-      auto* str = info->access_control != ast::AccessControl::kInvalid
+      auto* str = info->access_control
                       ? info->type->UnwrapRef()->As<sem::Struct>()
                       : nullptr;
 
@@ -740,7 +733,7 @@
 
   if (auto* r = storage_type->As<sem::MultisampledTexture>()) {
     if (r->dim() != ast::TextureDimension::k2d) {
-      diagnostics_.add_error("Only 2d multisampled textures are supported",
+      diagnostics_.add_error("only 2d multisampled textures are supported",
                              var->source());
       return false;
     }
@@ -754,24 +747,18 @@
   }
 
   if (auto* storage_tex = info->type->UnwrapRef()->As<sem::StorageTexture>()) {
-    if (storage_tex->access_control() == ast::AccessControl::kInvalid) {
-      diagnostics_.add_error("Storage Textures must have access control.",
-                             var->source());
-      return false;
-    }
-
-    if (info->access_control == ast::AccessControl::kReadWrite) {
+    if (info->access_control->access_control() ==
+        ast::AccessControl::kReadWrite) {
       diagnostics_.add_error(
-          "Storage Textures only support Read-Only and Write-Only access "
-          "control.",
+          "storage textures only support read-only and write-only access",
           var->source());
       return false;
     }
 
     if (!IsValidStorageTextureDimension(storage_tex->dim())) {
       diagnostics_.add_error(
-          "Cube dimensions for storage textures are not "
-          "supported.",
+          "cube dimensions for storage textures are not "
+          "supported",
           var->source());
       return false;
     }
@@ -2548,7 +2535,9 @@
       sem_var = builder_->create<sem::Variable>(var, info->type, constant_id);
     } else {
       sem_var = builder_->create<sem::Variable>(
-          var, info->type, info->storage_class, info->access_control);
+          var, info->type, info->storage_class,
+          info->access_control ? info->access_control->access_control()
+                               : ast::AccessControl::kReadWrite);
     }
 
     std::vector<const sem::VariableUser*> users;
@@ -3235,7 +3224,7 @@
                                      sem::Type* ty,
                                      const std::string& tn,
                                      ast::StorageClass sc,
-                                     ast::AccessControl::Access ac)
+                                     const ast::AccessControl* ac)
     : declaration(decl),
       type(ty),
       type_name(tn),
diff --git a/src/resolver/resolver.h b/src/resolver/resolver.h
index 2df4789..402e051 100644
--- a/src/resolver/resolver.h
+++ b/src/resolver/resolver.h
@@ -88,14 +88,14 @@
                  sem::Type* type,
                  const std::string& type_name,
                  ast::StorageClass storage_class,
-                 ast::AccessControl::Access ac);
+                 const ast::AccessControl* ac);
     ~VariableInfo();
 
     ast::Variable const* const declaration;
     sem::Type* type;
     std::string const type_name;
     ast::StorageClass storage_class;
-    ast::AccessControl::Access const access_control;
+    ast::AccessControl const* const access_control;
     std::vector<ast::IdentifierExpression*> users;
     sem::BindingPoint binding_point;
   };
@@ -382,7 +382,7 @@
 
   FunctionInfo* current_function_ = nullptr;
   sem::Statement* current_statement_ = nullptr;
-  const ast::AccessControl* curent_access_control_ = nullptr;
+  const ast::AccessControl* current_access_control_ = nullptr;
   BlockAllocator<VariableInfo> variable_infos_;
   BlockAllocator<FunctionInfo> function_infos_;
 };
diff --git a/src/resolver/type_validation_test.cc b/src/resolver/type_validation_test.cc
index 86481e9..29365ae 100644
--- a/src/resolver/type_validation_test.cc
+++ b/src/resolver/type_validation_test.cc
@@ -430,7 +430,7 @@
 using MultisampledTextureDimensionTest = ResolverTestWithParam<DimensionParams>;
 TEST_P(MultisampledTextureDimensionTest, All) {
   auto& params = GetParam();
-  Global("a", ty.multisampled_texture(params.dim, ty.i32()),
+  Global(Source{{12, 34}}, "a", ty.multisampled_texture(params.dim, ty.i32()),
          ast::StorageClass::kNone, nullptr,
          ast::DecorationList{
              create<ast::BindingDecoration>(0),
@@ -441,6 +441,8 @@
     EXPECT_TRUE(r()->Resolve()) << r()->error();
   } else {
     EXPECT_FALSE(r()->Resolve());
+    EXPECT_EQ(r()->error(),
+              "12:34 error: only 2d multisampled textures are supported");
   }
 }
 INSTANTIATE_TEST_SUITE_P(ResolverTypeValidationTest,
@@ -473,7 +475,7 @@
 TEST_P(MultisampledTextureTypeTest, All) {
   auto& params = GetParam();
   Global(
-      "a",
+      Source{{12, 34}}, "a",
       ty.multisampled_texture(ast::TextureDimension::k2d, params.type_func(ty)),
       ast::StorageClass::kNone, nullptr,
       ast::DecorationList{
@@ -485,6 +487,9 @@
     EXPECT_TRUE(r()->Resolve()) << r()->error();
   } else {
     EXPECT_FALSE(r()->Resolve());
+    EXPECT_EQ(r()->error(),
+              "12:34 error: texture_multisampled_2d<type>: type must be f32, "
+              "i32 or u32");
   }
 }
 INSTANTIATE_TEST_SUITE_P(ResolverTypeValidationTest,
@@ -516,7 +521,7 @@
   auto* st = ty.storage_texture(params.dim, ast::ImageFormat::kR32Uint);
   auto* ac = ty.access(ast::AccessControl::kReadOnly, st);
 
-  Global("a", ac, ast::StorageClass::kNone, nullptr,
+  Global(Source{{12, 34}}, "a", ac, ast::StorageClass::kNone, nullptr,
          ast::DecorationList{
              create<ast::BindingDecoration>(0),
              create<ast::GroupDecoration>(0),
@@ -526,6 +531,9 @@
     EXPECT_TRUE(r()->Resolve()) << r()->error();
   } else {
     EXPECT_FALSE(r()->Resolve());
+    EXPECT_EQ(
+        r()->error(),
+        "12:34 error: cube dimensions for storage textures are not supported");
   }
 }
 INSTANTIATE_TEST_SUITE_P(ResolverTypeValidationTest,
@@ -588,7 +596,7 @@
 
   auto* st_a = ty.storage_texture(ast::TextureDimension::k1d, params.format);
   auto* ac_a = ty.access(ast::AccessControl::kReadOnly, st_a);
-  Global("a", ac_a, ast::StorageClass::kNone, nullptr,
+  Global(Source{{12, 34}}, "a", ac_a, ast::StorageClass::kNone, nullptr,
          ast::DecorationList{
              create<ast::BindingDecoration>(0),
              create<ast::GroupDecoration>(0),
@@ -623,6 +631,10 @@
     EXPECT_TRUE(r()->Resolve()) << r()->error();
   } else {
     EXPECT_FALSE(r()->Resolve());
+    EXPECT_EQ(r()->error(),
+              "12:34 error: image format must be one of the texel formats "
+              "specified for storage textues in "
+              "https://gpuweb.github.io/gpuweb/wgsl/#texel-formats");
   }
 }
 INSTANTIATE_TEST_SUITE_P(ResolverTypeValidationTest,
@@ -635,7 +647,7 @@
   // [[group(0), binding(0)]]
   // var a : texture_storage_1d<ru32int>;
 
-  auto* st = ty.storage_texture(ast::TextureDimension::k1d,
+  auto* st = ty.storage_texture(Source{{12, 34}}, ast::TextureDimension::k1d,
                                 ast::ImageFormat::kR32Uint);
 
   Global("a", st, ast::StorageClass::kNone, nullptr,
@@ -645,6 +657,8 @@
          });
 
   EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(r()->error(),
+            "12:34 error: storage textures must have access control");
 }
 
 TEST_F(StorageTextureAccessControlTest, RWAccessControl_Fail) {
@@ -653,14 +667,18 @@
 
   auto* st = ty.storage_texture(ast::TextureDimension::k1d,
                                 ast::ImageFormat::kR32Uint);
+  auto* ac = ty.access(ast::AccessControl::kReadWrite, st);
 
-  Global("a", st, ast::StorageClass::kNone, nullptr,
+  Global(Source{{12, 34}}, "a", ac, ast::StorageClass::kNone, nullptr,
          ast::DecorationList{
              create<ast::BindingDecoration>(0),
              create<ast::GroupDecoration>(0),
          });
 
   EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(r()->error(),
+            "12:34 error: storage textures only support read-only and "
+            "write-only access");
 }
 
 TEST_F(StorageTextureAccessControlTest, ReadOnlyAccessControl_Pass) {
diff --git a/src/sem/variable.cc b/src/sem/variable.cc
index 3700c06..a8cd011 100644
--- a/src/sem/variable.cc
+++ b/src/sem/variable.cc
@@ -39,7 +39,7 @@
     : declaration_(declaration),
       type_(type),
       storage_class_(ast::StorageClass::kNone),
-      access_control_(ast::AccessControl::kInvalid),
+      access_control_(ast::AccessControl::kReadWrite),
       is_pipeline_constant_(true),
       constant_id_(constant_id) {}
 
diff --git a/src/transform/decompose_storage_access.cc b/src/transform/decompose_storage_access.cc
index 46495a1..e718b13 100644
--- a/src/transform/decompose_storage_access.cc
+++ b/src/transform/decompose_storage_access.cc
@@ -364,7 +364,7 @@
                                        const sem::VariableUser* var_user,
                                        ast::Type* ty) {
   if (var_user &&
-      var_user->Variable()->AccessControl() != ast::AccessControl::kInvalid) {
+      var_user->Variable()->StorageClass() == ast::StorageClass::kStorage) {
     return ctx->dst->create<ast::AccessControl>(
         var_user->Variable()->AccessControl(), ty);
   }
diff --git a/src/writer/hlsl/generator_impl.cc b/src/writer/hlsl/generator_impl.cc
index 626cc77..5a3621b 100644
--- a/src/writer/hlsl/generator_impl.cc
+++ b/src/writer/hlsl/generator_impl.cc
@@ -249,7 +249,7 @@
 
   out << "as";
   if (!EmitType(out, type, ast::StorageClass::kNone,
-                ast::AccessControl::kInvalid, "")) {
+                ast::AccessControl::kReadWrite, "")) {
     return false;
   }
   out << "(";
@@ -1309,7 +1309,7 @@
     out << "{";
   } else {
     if (!EmitType(out, type, ast::StorageClass::kNone,
-                  ast::AccessControl::kInvalid, "")) {
+                  ast::AccessControl::kReadWrite, "")) {
       return false;
     }
     out << "(";
@@ -1571,7 +1571,7 @@
   auto* func = builder_.Sem().Get(func_ast);
 
   if (!EmitType(out, func->ReturnType(), ast::StorageClass::kNone,
-                ast::AccessControl::kInvalid, "")) {
+                ast::AccessControl::kReadWrite, "")) {
     return false;
   }
 
@@ -1750,11 +1750,6 @@
       continue;  // Global already emitted
     }
 
-    if (var->AccessControl() == ast::AccessControl::kInvalid) {
-      diagnostics_.add_error("access control type required for storage buffer");
-      return false;
-    }
-
     auto* type = var->Type()->UnwrapRef();
     if (!EmitType(out, type, ast::StorageClass::kStorage, var->AccessControl(),
                   "")) {
@@ -2129,7 +2124,7 @@
     out << "0u";
   } else if (auto* vec = type->As<sem::Vector>()) {
     if (!EmitType(out, type, ast::StorageClass::kNone,
-                  ast::AccessControl::kInvalid, "")) {
+                  ast::AccessControl::kReadWrite, "")) {
       return false;
     }
     ScopedParen sp(out);
@@ -2143,7 +2138,7 @@
     }
   } else if (auto* mat = type->As<sem::Matrix>()) {
     if (!EmitType(out, type, ast::StorageClass::kNone,
-                  ast::AccessControl::kInvalid, "")) {
+                  ast::AccessControl::kReadWrite, "")) {
       return false;
     }
     ScopedParen sp(out);
@@ -2515,7 +2510,7 @@
 
     auto mem_name = builder_.Symbols().NameFor(mem->Declaration()->symbol());
     if (!EmitType(out, mem->Type(), ast::StorageClass::kNone,
-                  ast::AccessControl::kInvalid, mem_name)) {
+                  ast::AccessControl::kReadWrite, mem_name)) {
       return false;
     }
     // Array member name will be output with the type
diff --git a/src/writer/hlsl/generator_impl_type_test.cc b/src/writer/hlsl/generator_impl_type_test.cc
index a4c2b97..1346cc8 100644
--- a/src/writer/hlsl/generator_impl_type_test.cc
+++ b/src/writer/hlsl/generator_impl_type_test.cc
@@ -38,7 +38,7 @@
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone,
-                           ast::AccessControl::kInvalid, "ary"))
+                           ast::AccessControl::kReadWrite, "ary"))
       << gen.error();
   EXPECT_EQ(result(), "bool ary[4]");
 }
@@ -50,7 +50,7 @@
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone,
-                           ast::AccessControl::kInvalid, "ary"))
+                           ast::AccessControl::kReadWrite, "ary"))
       << gen.error();
   EXPECT_EQ(result(), "bool ary[5][4]");
 }
@@ -64,7 +64,7 @@
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone,
-                           ast::AccessControl::kInvalid, "ary"))
+                           ast::AccessControl::kReadWrite, "ary"))
       << gen.error();
   EXPECT_EQ(result(), "bool ary[5][4][1]");
 }
@@ -76,7 +76,7 @@
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone,
-                           ast::AccessControl::kInvalid, "ary"))
+                           ast::AccessControl::kReadWrite, "ary"))
       << gen.error();
   EXPECT_EQ(result(), "bool ary[6][5][4]");
 }
@@ -88,7 +88,7 @@
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone,
-                           ast::AccessControl::kInvalid, ""))
+                           ast::AccessControl::kReadWrite, ""))
       << gen.error();
   EXPECT_EQ(result(), "bool[4]");
 }
@@ -100,7 +100,7 @@
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone,
-                           ast::AccessControl::kInvalid, "ary"))
+                           ast::AccessControl::kReadWrite, "ary"))
       << gen.error();
   EXPECT_EQ(result(), "bool ary[]");
 }
@@ -111,7 +111,7 @@
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.EmitType(out, bool_, ast::StorageClass::kNone,
-                           ast::AccessControl::kInvalid, ""))
+                           ast::AccessControl::kReadWrite, ""))
       << gen.error();
   EXPECT_EQ(result(), "bool");
 }
@@ -122,7 +122,7 @@
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.EmitType(out, f32, ast::StorageClass::kNone,
-                           ast::AccessControl::kInvalid, ""))
+                           ast::AccessControl::kReadWrite, ""))
       << gen.error();
   EXPECT_EQ(result(), "float");
 }
@@ -133,7 +133,7 @@
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.EmitType(out, i32, ast::StorageClass::kNone,
-                           ast::AccessControl::kInvalid, ""))
+                           ast::AccessControl::kReadWrite, ""))
       << gen.error();
   EXPECT_EQ(result(), "int");
 }
@@ -146,7 +146,7 @@
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.EmitType(out, mat2x3, ast::StorageClass::kNone,
-                           ast::AccessControl::kInvalid, ""))
+                           ast::AccessControl::kReadWrite, ""))
       << gen.error();
   EXPECT_EQ(result(), "float2x3");
 }
@@ -159,7 +159,7 @@
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.EmitType(out, p, ast::StorageClass::kNone,
-                           ast::AccessControl::kInvalid, ""))
+                           ast::AccessControl::kReadWrite, ""))
       << gen.error();
   EXPECT_EQ(result(), "float*");
 }
@@ -214,7 +214,7 @@
 
   auto* sem_s = program->TypeOf(s)->As<sem::Struct>();
   ASSERT_TRUE(gen.EmitType(out, sem_s, ast::StorageClass::kNone,
-                           ast::AccessControl::kInvalid, ""))
+                           ast::AccessControl::kReadWrite, ""))
       << gen.error();
   EXPECT_EQ(result(), "S");
 }
@@ -234,7 +234,7 @@
 
   auto* sem_s = program->TypeOf(s)->As<sem::Struct>();
   ASSERT_TRUE(gen.EmitType(out, sem_s, ast::StorageClass::kNone,
-                           ast::AccessControl::kInvalid, ""))
+                           ast::AccessControl::kReadWrite, ""))
       << gen.error();
   EXPECT_EQ(gen.result(), R"(struct S {
   int a;
@@ -290,7 +290,7 @@
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.EmitType(out, u32, ast::StorageClass::kNone,
-                           ast::AccessControl::kInvalid, ""))
+                           ast::AccessControl::kReadWrite, ""))
       << gen.error();
   EXPECT_EQ(result(), "uint");
 }
@@ -302,7 +302,7 @@
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.EmitType(out, vec3, ast::StorageClass::kNone,
-                           ast::AccessControl::kInvalid, ""))
+                           ast::AccessControl::kReadWrite, ""))
       << gen.error();
   EXPECT_EQ(result(), "float3");
 }
@@ -313,7 +313,7 @@
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.EmitType(out, void_, ast::StorageClass::kNone,
-                           ast::AccessControl::kInvalid, ""))
+                           ast::AccessControl::kReadWrite, ""))
       << gen.error();
   EXPECT_EQ(result(), "void");
 }
@@ -324,7 +324,7 @@
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.EmitType(out, sampler, ast::StorageClass::kNone,
-                           ast::AccessControl::kInvalid, ""))
+                           ast::AccessControl::kReadWrite, ""))
       << gen.error();
   EXPECT_EQ(result(), "SamplerState");
 }
@@ -335,7 +335,7 @@
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.EmitType(out, sampler, ast::StorageClass::kNone,
-                           ast::AccessControl::kInvalid, ""))
+                           ast::AccessControl::kReadWrite, ""))
       << gen.error();
   EXPECT_EQ(result(), "SamplerComparisonState");
 }
@@ -532,7 +532,7 @@
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.EmitType(out, s, ast::StorageClass::kNone,
-                           ast::AccessControl::kInvalid, ""))
+                           ast::AccessControl::kReadWrite, ""))
       << gen.error();
   EXPECT_EQ(result(), "Texture2DMS<float4>");
 }
diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc
index f267f06..e10bab5 100644
--- a/src/writer/spirv/builder.cc
+++ b/src/writer/spirv/builder.cc
@@ -784,35 +784,35 @@
 
   if (var->has_constructor()) {
     ops.push_back(Operand::Int(init_id));
-  } else if (sem->AccessControl() != ast::AccessControl::kInvalid) {
-    // type is a sem::Struct or a sem::StorageTexture
-    switch (sem->AccessControl()) {
-      case ast::AccessControl::kInvalid:
-        TINT_ICE(builder_.Diagnostics()) << "missing access control";
-        break;
-      case ast::AccessControl::kWriteOnly:
-        push_annot(
-            spv::Op::OpDecorate,
-            {Operand::Int(var_id), Operand::Int(SpvDecorationNonReadable)});
-        break;
-      case ast::AccessControl::kReadOnly:
-        push_annot(
-            spv::Op::OpDecorate,
-            {Operand::Int(var_id), Operand::Int(SpvDecorationNonWritable)});
-        break;
-      case ast::AccessControl::kReadWrite:
-        break;
-    }
-  } else if (!type->Is<sem::Sampler>()) {
-    // If we don't have a constructor and we're an Output or Private variable,
-    // then WGSL requires that we zero-initialize.
-    if (sem->StorageClass() == ast::StorageClass::kPrivate ||
-        sem->StorageClass() == ast::StorageClass::kOutput) {
-      init_id = GenerateConstantNullIfNeeded(type);
-      if (init_id == 0) {
-        return 0;
+  } else {
+    if (type->Is<sem::StorageTexture>() || type->Is<sem::Struct>()) {
+      // type is a sem::Struct or a sem::StorageTexture
+      switch (sem->AccessControl()) {
+        case ast::AccessControl::kWriteOnly:
+          push_annot(
+              spv::Op::OpDecorate,
+              {Operand::Int(var_id), Operand::Int(SpvDecorationNonReadable)});
+          break;
+        case ast::AccessControl::kReadOnly:
+          push_annot(
+              spv::Op::OpDecorate,
+              {Operand::Int(var_id), Operand::Int(SpvDecorationNonWritable)});
+          break;
+        case ast::AccessControl::kReadWrite:
+          break;
       }
-      ops.push_back(Operand::Int(init_id));
+    }
+    if (!type->Is<sem::Sampler>()) {
+      // If we don't have a constructor and we're an Output or Private
+      // variable, then WGSL requires that we zero-initialize.
+      if (sem->StorageClass() == ast::StorageClass::kPrivate ||
+          sem->StorageClass() == ast::StorageClass::kOutput) {
+        init_id = GenerateConstantNullIfNeeded(type);
+        if (init_id == 0) {
+          return 0;
+        }
+        ops.push_back(Operand::Int(init_id));
+      }
     }
   }