Remove sem::AccessControl

In preparation for implementing
https://github.com/gpuweb/gpuweb/issues/1604, this change removes the
sem::AccessControl node. Instead, the ast::AccessControl::Access enum is
now on the sem::StorageTexture class, as well as on sem::Variable. For
sem::Variable, the field is set when the variable's type is either a
storage buffer or a storage texture.

Bug: tint:802
Change-Id: Id479af36b401d067b015027923f4e715f5f69f25
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/51020
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>

src/sem/access_control_type.cc

@@ -1,70 +0,0 @@
-// Copyright 2020 The Tint Authors.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#include "src/sem/access_control_type.h"
-
-#include "src/program_builder.h"
-
-TINT_INSTANTIATE_TYPEINFO(tint::sem::AccessControl);
-
-namespace tint {
-namespace sem {
-
-AccessControl::AccessControl(ast::AccessControl::Access access,
-                             const Type* subtype)
-    : access_(access), subtype_(subtype) {
-  TINT_ASSERT(subtype_);
-  TINT_ASSERT(!subtype_->Is<AccessControl>());
-}
-
-AccessControl::AccessControl(AccessControl&&) = default;
-
-AccessControl::~AccessControl() = default;
-
-std::string AccessControl::type_name() const {
-  std::string name = "__access_control_";
-  switch (access_) {
-    case ast::AccessControl::kReadOnly:
-      name += "read_only";
-      break;
-    case ast::AccessControl::kWriteOnly:
-      name += "write_only";
-      break;
-    case ast::AccessControl::kReadWrite:
-      name += "read_write";
-      break;
-  }
-  return name + subtype_->type_name();
-}
-
-std::string AccessControl::FriendlyName(const SymbolTable& symbols) const {
-  std::ostringstream out;
-  out << "[[access(";
-  switch (access_) {
-    case ast::AccessControl::kReadOnly:
-      out << "read";
-      break;
-    case ast::AccessControl::kWriteOnly:
-      out << "write";
-      break;
-    case ast::AccessControl::kReadWrite:
-      out << "read_write";
-      break;
-  }
-  out << ")]] " << subtype_->FriendlyName(symbols);
-  return out.str();
-}
-
-}  // namespace sem
-}  // namespace tint

src/sem/access_control_type.h

@@ -1,65 +0,0 @@
-// Copyright 2020 The Tint Authors.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#ifndef SRC_SEM_ACCESS_CONTROL_TYPE_H_
-#define SRC_SEM_ACCESS_CONTROL_TYPE_H_
-
-#include <string>
-
-#include "src/ast/access_control.h"
-#include "src/sem/type.h"
-
-namespace tint {
-namespace sem {
-
-/// An access control type. Holds an access setting and pointer to another type.
-class AccessControl : public Castable<AccessControl, Type> {
- public:
-  /// Constructor
-  /// @param access the access control setting
-  /// @param subtype the access controlled type
-  AccessControl(ast::AccessControl::Access access, const Type* subtype);
-  /// Move constructor
-  AccessControl(AccessControl&&);
-  ~AccessControl() override;
-
-  /// @returns true if the access control is read only
-  bool IsReadOnly() const { return access_ == ast::AccessControl::kReadOnly; }
-  /// @returns true if the access control is write only
-  bool IsWriteOnly() const { return access_ == ast::AccessControl::kWriteOnly; }
-  /// @returns true if the access control is read/write
-  bool IsReadWrite() const { return access_ == ast::AccessControl::kReadWrite; }
-
-  /// @returns the access control value
-  ast::AccessControl::Access access_control() const { return access_; }
-  /// @returns the subtype type
-  Type* type() const { return const_cast<Type*>(subtype_); }
-
-  /// @returns the name for this type
-  std::string type_name() const override;
-
-  /// @param symbols the program's symbol table
-  /// @returns the name for this type that closely resembles how it would be
-  /// declared in WGSL.
-  std::string FriendlyName(const SymbolTable& symbols) const override;
-
- private:
-  ast::AccessControl::Access const access_;
-  const Type* const subtype_;
-};
-
-}  // namespace sem
-}  // namespace tint
-
-#endif  // SRC_SEM_ACCESS_CONTROL_TYPE_H_

src/sem/access_control_type_test.cc

@@ -1,80 +0,0 @@
-// Copyright 2020 The Tint Authors.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#include "src/sem/access_control_type.h"
-
-#include "src/sem/test_helper.h"
-#include "src/sem/texture_type.h"
-
-namespace tint {
-namespace sem {
-namespace {
-
-using AccessControlTest = TestHelper;
-
-TEST_F(AccessControlTest, Create) {
-  U32 u32;
-  AccessControl a{ast::AccessControl::kReadWrite, &u32};
-  EXPECT_TRUE(a.IsReadWrite());
-  EXPECT_EQ(a.type(), &u32);
-}
-
-TEST_F(AccessControlTest, AccessRead) {
-  I32 i32;
-  AccessControl at{ast::AccessControl::kReadOnly, &i32};
-  EXPECT_TRUE(at.IsReadOnly());
-  EXPECT_FALSE(at.IsWriteOnly());
-  EXPECT_FALSE(at.IsReadWrite());
-
-  EXPECT_EQ(at.type_name(), "__access_control_read_only__i32");
-}
-
-TEST_F(AccessControlTest, AccessWrite) {
-  I32 i32;
-  AccessControl at{ast::AccessControl::kWriteOnly, &i32};
-  EXPECT_FALSE(at.IsReadOnly());
-  EXPECT_TRUE(at.IsWriteOnly());
-  EXPECT_FALSE(at.IsReadWrite());
-
-  EXPECT_EQ(at.type_name(), "__access_control_write_only__i32");
-}
-
-TEST_F(AccessControlTest, AccessReadWrite) {
-  I32 i32;
-  AccessControl at{ast::AccessControl::kReadWrite, &i32};
-  EXPECT_FALSE(at.IsReadOnly());
-  EXPECT_FALSE(at.IsWriteOnly());
-  EXPECT_TRUE(at.IsReadWrite());
-
-  EXPECT_EQ(at.type_name(), "__access_control_read_write__i32");
-}
-
-TEST_F(AccessControlTest, FriendlyNameReadOnly) {
-  AccessControl at{ast::AccessControl::kReadOnly, ty.i32()};
-  EXPECT_EQ(at.FriendlyName(Symbols()), "[[access(read)]] i32");
-}
-
-TEST_F(AccessControlTest, FriendlyNameWriteOnly) {
-  AccessControl at{ast::AccessControl::kWriteOnly, ty.i32()};
-  EXPECT_EQ(at.FriendlyName(Symbols()), "[[access(write)]] i32");
-}
-
-TEST_F(AccessControlTest, FriendlyNameReadWrite) {
-  AccessControl at{ast::AccessControl::kReadWrite, ty.i32()};
-  EXPECT_EQ(at.FriendlyName(Symbols()), "[[access(read_write)]] i32");
-}
-
-}  // namespace
-}  // namespace sem
-}  // namespace tint

src/sem/bool_type_test.cc

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "src/sem/access_control_type.h"
 #include "src/sem/test_helper.h"
 #include "src/sem/texture_type.h"
 

src/sem/depth_texture_type_test.cc

@@ -16,7 +16,6 @@
 
 #include "src/sem/test_helper.h"
 
-#include "src/sem/access_control_type.h"
 #include "src/sem/external_texture_type.h"
 #include "src/sem/sampled_texture_type.h"
 #include "src/sem/storage_texture_type.h"

src/sem/external_texture_type_test.cc

@@ -14,7 +14,6 @@
 
 #include "src/sem/external_texture_type.h"
 
-#include "src/sem/access_control_type.h"
 #include "src/sem/depth_texture_type.h"
 #include "src/sem/multisampled_texture_type.h"
 #include "src/sem/sampled_texture_type.h"

src/sem/f32_type_test.cc

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "src/sem/access_control_type.h"
 #include "src/sem/test_helper.h"
 #include "src/sem/texture_type.h"
 

src/sem/i32_type_test.cc

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "src/sem/access_control_type.h"
 #include "src/sem/test_helper.h"
 #include "src/sem/texture_type.h"
 

src/sem/matrix_type_test.cc

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "src/sem/access_control_type.h"
 #include "src/sem/test_helper.h"
 #include "src/sem/texture_type.h"
 

src/sem/multisampled_texture_type_test.cc

@@ -14,7 +14,6 @@
 
 #include "src/sem/multisampled_texture_type.h"
 
-#include "src/sem/access_control_type.h"
 #include "src/sem/depth_texture_type.h"
 #include "src/sem/external_texture_type.h"
 #include "src/sem/sampled_texture_type.h"

src/sem/pointer_type_test.cc

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "src/sem/access_control_type.h"
 #include "src/sem/test_helper.h"
 #include "src/sem/texture_type.h"
 

src/sem/sampled_texture_type_test.cc

@@ -14,7 +14,6 @@
 
 #include "src/sem/sampled_texture_type.h"
 
-#include "src/sem/access_control_type.h"
 #include "src/sem/depth_texture_type.h"
 #include "src/sem/external_texture_type.h"
 #include "src/sem/storage_texture_type.h"

src/sem/sampler_type_test.cc

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "src/sem/access_control_type.h"
 #include "src/sem/test_helper.h"
 #include "src/sem/texture_type.h"
 

src/sem/sem_array_test.cc

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "src/sem/access_control_type.h"
 #include "src/sem/test_helper.h"
 #include "src/sem/texture_type.h"
 

src/sem/sem_struct_test.cc

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "src/sem/access_control_type.h"
 #include "src/sem/struct.h"
 #include "src/sem/test_helper.h"
 #include "src/sem/texture_type.h"

src/sem/storage_texture_type.cc

@@ -23,8 +23,12 @@ namespace sem {
 
 StorageTexture::StorageTexture(ast::TextureDimension dim,
                                ast::ImageFormat format,
+                               ast::AccessControl::Access access_control,
                                sem::Type* subtype)
-    : Base(dim), image_format_(format), subtype_(subtype) {}
+    : Base(dim),
+      image_format_(format),
+      access_control_(access_control),
+      subtype_(subtype) {}
 
 StorageTexture::StorageTexture(StorageTexture&&) = default;
 
@@ -32,13 +36,15 @@ StorageTexture::~StorageTexture() = default;
 
 std::string StorageTexture::type_name() const {
   std::ostringstream out;
-  out << "__storage_texture_" << dim() << "_" << image_format_;
+  out << "__storage_texture_" << dim() << "_" << image_format_ << "_"
+      << access_control_;
   return out.str();
 }
 
 std::string StorageTexture::FriendlyName(const SymbolTable&) const {
   std::ostringstream out;
-  out << "texture_storage_" << dim() << "<" << image_format_ << ">";
+  out << "texture_storage_" << dim() << "<" << image_format_ << ", "
+      << access_control_ << ">";
   return out.str();
 }
 

src/sem/storage_texture_type.h

@@ -17,6 +17,7 @@
 
 #include <string>
 
+#include "src/ast/access_control.h"
 #include "src/ast/storage_texture.h"
 #include "src/sem/texture_type.h"
 
@@ -31,9 +32,11 @@ class StorageTexture : public Castable<StorageTexture, Texture> {
   /// Constructor
   /// @param dim the dimensionality of the texture
   /// @param format the image format of the texture
+  /// @param access_control the access control type of the texture
   /// @param subtype the storage subtype. Use SubtypeFor() to calculate this.
   StorageTexture(ast::TextureDimension dim,
                  ast::ImageFormat format,
+                 ast::AccessControl::Access access_control,
                  sem::Type* subtype);
 
   /// Move constructor
@@ -46,6 +49,9 @@ class StorageTexture : public Castable<StorageTexture, Texture> {
   /// @returns the image format
   ast::ImageFormat image_format() const { return image_format_; }
 
+  /// @returns the access control
+  ast::AccessControl::Access access_control() const { return access_control_; }
+
   /// @returns the name for this type
   std::string type_name() const override;
 
@@ -61,6 +67,7 @@ class StorageTexture : public Castable<StorageTexture, Texture> {
 
  private:
   ast::ImageFormat const image_format_;
+  ast::AccessControl::Access const access_control_;
   Type* const subtype_;
 };
 

src/sem/storage_texture_type_test.cc

@@ -14,7 +14,6 @@
 
 #include "src/sem/storage_texture_type.h"
 
-#include "src/sem/access_control_type.h"
 #include "src/sem/depth_texture_type.h"
 #include "src/sem/external_texture_type.h"
 #include "src/sem/sampled_texture_type.h"
@@ -30,7 +29,8 @@ TEST_F(StorageTextureTest, Dim) {
   auto* subtype =
       StorageTexture::SubtypeFor(ast::ImageFormat::kRgba32Float, Types());
   auto* s = create<StorageTexture>(ast::TextureDimension::k2dArray,
-                                   ast::ImageFormat::kRgba32Float, subtype);
+                                   ast::ImageFormat::kRgba32Float,
+                                   ast::AccessControl::kReadWrite, subtype);
   EXPECT_EQ(s->dim(), ast::TextureDimension::k2dArray);
 }
 
@@ -38,7 +38,8 @@ TEST_F(StorageTextureTest, Format) {
   auto* subtype =
       StorageTexture::SubtypeFor(ast::ImageFormat::kRgba32Float, Types());
   auto* s = create<StorageTexture>(ast::TextureDimension::k2dArray,
-                                   ast::ImageFormat::kRgba32Float, subtype);
+                                   ast::ImageFormat::kRgba32Float,
+                                   ast::AccessControl::kReadWrite, subtype);
   EXPECT_EQ(s->image_format(), ast::ImageFormat::kRgba32Float);
 }
 
@@ -46,24 +47,28 @@ TEST_F(StorageTextureTest, TypeName) {
   auto* subtype =
       StorageTexture::SubtypeFor(ast::ImageFormat::kRgba32Float, Types());
   auto* s = create<StorageTexture>(ast::TextureDimension::k2dArray,
-                                   ast::ImageFormat::kRgba32Float, subtype);
-  EXPECT_EQ(s->type_name(), "__storage_texture_2d_array_rgba32float");
+                                   ast::ImageFormat::kRgba32Float,
+                                   ast::AccessControl::kReadWrite, subtype);
+  EXPECT_EQ(s->type_name(),
+            "__storage_texture_2d_array_rgba32float_read_write");
 }
 
 TEST_F(StorageTextureTest, FriendlyName) {
   auto* subtype =
       StorageTexture::SubtypeFor(ast::ImageFormat::kRgba32Float, Types());
   auto* s = create<StorageTexture>(ast::TextureDimension::k2dArray,
-                                   ast::ImageFormat::kRgba32Float, subtype);
+                                   ast::ImageFormat::kRgba32Float,
+                                   ast::AccessControl::kReadWrite, subtype);
   EXPECT_EQ(s->FriendlyName(Symbols()),
-            "texture_storage_2d_array<rgba32float>");
+            "texture_storage_2d_array<rgba32float, read_write>");
 }
 
 TEST_F(StorageTextureTest, F32) {
   auto* subtype =
       sem::StorageTexture::SubtypeFor(ast::ImageFormat::kRgba32Float, Types());
   Type* s = create<StorageTexture>(ast::TextureDimension::k2dArray,
-                                   ast::ImageFormat::kRgba32Float, subtype);
+                                   ast::ImageFormat::kRgba32Float,
+                                   ast::AccessControl::kReadWrite, subtype);
 
   auto program = Build();
 
@@ -77,7 +82,8 @@ TEST_F(StorageTextureTest, U32) {
   auto* subtype =
       sem::StorageTexture::SubtypeFor(ast::ImageFormat::kRg32Uint, Types());
   Type* s = create<StorageTexture>(ast::TextureDimension::k2dArray,
-                                   ast::ImageFormat::kRg32Uint, subtype);
+                                   ast::ImageFormat::kRg32Uint,
+                                   ast::AccessControl::kReadWrite, subtype);
 
   auto program = Build();
 
@@ -91,7 +97,8 @@ TEST_F(StorageTextureTest, I32) {
   auto* subtype =
       sem::StorageTexture::SubtypeFor(ast::ImageFormat::kRgba32Sint, Types());
   Type* s = create<StorageTexture>(ast::TextureDimension::k2dArray,
-                                   ast::ImageFormat::kRgba32Sint, subtype);
+                                   ast::ImageFormat::kRgba32Sint,
+                                   ast::AccessControl::kReadWrite, subtype);
 
   auto program = Build();
 

src/sem/type.cc

@@ -14,7 +14,6 @@
 
 #include "src/sem/type.h"
 
-#include "src/sem/access_control_type.h"
 #include "src/sem/bool_type.h"
 #include "src/sem/f32_type.h"
 #include "src/sem/i32_type.h"
@@ -45,10 +44,8 @@ const Type* Type::UnwrapPtr() const {
 }
 
 const Type* Type::UnwrapAccess() const {
+  // TODO(amaiorano): Delete this function
   auto* type = this;
-  while (auto* access = type->As<sem::AccessControl>()) {
-    type = access->type();
-  }
   return type;
 }
 
@@ -57,8 +54,6 @@ const Type* Type::UnwrapAll() const {
   while (true) {
     if (auto* ptr = type->As<sem::Pointer>()) {
       type = ptr->type();
-    } else if (auto* access = type->As<sem::AccessControl>()) {
-      type = access->type();
     } else {
       break;
     }

src/sem/u32_type_test.cc

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "src/sem/access_control_type.h"
 #include "src/sem/test_helper.h"
 #include "src/sem/texture_type.h"
 

src/sem/variable.cc

@@ -25,10 +25,12 @@ namespace sem {
 
 Variable::Variable(const ast::Variable* declaration,
                    const sem::Type* type,
-                   ast::StorageClass storage_class)
+                   ast::StorageClass storage_class,
+                   ast::AccessControl::Access access_control)
     : declaration_(declaration),
       type_(type),
       storage_class_(storage_class),
+      access_control_(access_control),
       is_pipeline_constant_(false) {}
 
 Variable::Variable(const ast::Variable* declaration,
@@ -37,6 +39,7 @@ Variable::Variable(const ast::Variable* declaration,
     : declaration_(declaration),
       type_(type),
       storage_class_(ast::StorageClass::kNone),
+      access_control_(ast::AccessControl::kInvalid),
       is_pipeline_constant_(true),
       constant_id_(constant_id) {}
 

src/sem/variable.h

@@ -17,6 +17,7 @@
 
 #include <vector>
 
+#include "src/ast/access_control.h"
 #include "src/ast/storage_class.h"
 #include "src/sem/expression.h"
 
@@ -41,9 +42,11 @@ class Variable : public Castable<Variable, Node> {
   /// @param declaration the AST declaration node
   /// @param type the variable type
   /// @param storage_class the variable storage class
+  /// @param access_control the variable access control type
   Variable(const ast::Variable* declaration,
            const sem::Type* type,
-           ast::StorageClass storage_class);
+           ast::StorageClass storage_class,
+           ast::AccessControl::Access access_control);
 
   /// Constructor for overridable pipeline constants
   /// @param declaration the AST declaration node
@@ -65,6 +68,9 @@ class Variable : public Castable<Variable, Node> {
   /// @returns the storage class for the variable
   ast::StorageClass StorageClass() const { return storage_class_; }
 
+  /// @returns the access control for the variable
+  ast::AccessControl::Access AccessControl() const { return access_control_; }
+
   /// @returns the expressions that use the variable
   const std::vector<const VariableUser*>& Users() const { return users_; }
 
@@ -81,6 +87,7 @@ class Variable : public Castable<Variable, Node> {
   const ast::Variable* const declaration_;
   const sem::Type* const type_;
   ast::StorageClass const storage_class_;
+  ast::AccessControl::Access const access_control_;
   std::vector<const VariableUser*> users_;
   const bool is_pipeline_constant_;
   const uint16_t constant_id_ = 0;

src/sem/vector_type_test.cc

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "src/sem/access_control_type.h"
 #include "src/sem/test_helper.h"
 #include "src/sem/texture_type.h"