From 4db10dd496602b916295ab4e49b3c738563c1db6 Mon Sep 17 00:00:00 2001
From: Ben Clayton <bclayton@google.com>
Date: Fri, 16 Apr 2021 08:47:14 +0000
Subject: [PATCH] Resolver: Handle AccessControl in Canonical()

The canonical type was stopping at the first encountered type::AccessControl, which meant the alias in access<alias<i32>> was not being unwrapped.

Bug: tint:705
Change-Id: Idcbd824808d8ee3098fb1861add5014d7d46b0ad
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47762
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
---
 src/program_builder.h                | 11 +++++++++++
 src/resolver/resolver.cc             |  7 ++++++-
 src/resolver/resolver_test_helper.h  |  6 ++++++
 src/resolver/type_validation_test.cc |  6 ++++++
 4 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/src/program_builder.h b/src/program_builder.h
index 6919904a8a..ce0f9cbcdd 100644
--- a/src/program_builder.h
+++ b/src/program_builder.h
@@ -43,6 +43,7 @@
 #include "src/ast/variable_decl_statement.h"
 #include "src/program.h"
 #include "src/program_id.h"
+#include "src/type/access_control_type.h"
 #include "src/type/alias_type.h"
 #include "src/type/array_type.h"
 #include "src/type/bool_type.h"
@@ -517,6 +518,7 @@ class ProgramBuilder {
     type::Array* array(uint32_t stride) const {
       return array(Of<T>(), N, stride);
     }
+
     /// Creates an alias type
     /// @param name the alias name
     /// @param type the alias type
@@ -527,6 +529,15 @@ class ProgramBuilder {
           builder->Sym(std::forward<NAME>(name)), type);
     }
 
+    /// Creates an access control qualifier type
+    /// @param access the access control
+    /// @param type the inner type
+    /// @returns the access control qualifier type
+    type::AccessControl* access(ast::AccessControl access,
+                                type::Type* type) const {
+      return builder->create<type::AccessControl>(access, type);
+    }
+
     /// @return the tint AST pointer to `type` with the given ast::StorageClass
     /// @param type the type of the pointer
     /// @param storage_class the storage class of the pointer
diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index 5bbc82093d..257c822a75 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -2279,9 +2279,10 @@ std::string Resolver::VectorPretty(uint32_t size, type::Type* element_type) {
 }
 
 type::Type* Resolver::Canonical(type::Type* type) {
-  using Type = type::Type;
+  using AccessControl = type::AccessControl;
   using Alias = type::Alias;
   using Matrix = type::Matrix;
+  using Type = type::Type;
   using Vector = type::Vector;
 
   std::function<Type*(Type*)> make_canonical;
@@ -2299,6 +2300,10 @@ type::Type* Resolver::Canonical(type::Type* type) {
       return builder_->create<Matrix>(make_canonical(m->type()), m->rows(),
                                       m->columns());
     }
+    if (auto* ac = ct->As<AccessControl>()) {
+      return builder_->create<AccessControl>(ac->access_control(),
+                                             make_canonical(ac->type()));
+    }
     return ct;
   };
 
diff --git a/src/resolver/resolver_test_helper.h b/src/resolver/resolver_test_helper.h
index a3dfce1ad1..3ac82f7f08 100644
--- a/src/resolver/resolver_test_helper.h
+++ b/src/resolver/resolver_test_helper.h
@@ -194,6 +194,12 @@ type::Type* ty_alias(const ProgramBuilder::TypesBuilder& ty) {
   return ty.alias("alias_" + type->type_name(), type);
 }
 
+template <create_type_func_ptr create_type>
+type::Type* ty_access(const ProgramBuilder::TypesBuilder& ty) {
+  auto* type = create_type(ty);
+  return ty.access(ast::AccessControl::kReadOnly, type);
+}
+
 }  // namespace resolver
 }  // namespace tint
 
diff --git a/src/resolver/type_validation_test.cc b/src/resolver/type_validation_test.cc
index 973c7182a8..213099b823 100644
--- a/src/resolver/type_validation_test.cc
+++ b/src/resolver/type_validation_test.cc
@@ -490,6 +490,12 @@ static constexpr Params cases[] = {
     Params{ty_alias<ty_alias<ty_mat3x3<ty_alias<ty_f32>>>>, ty_mat3x3<ty_f32>},
     Params{ty_alias<ty_alias<ty_mat3x3<ty_alias<ty_alias<ty_f32>>>>>,
            ty_mat3x3<ty_f32>},
+
+    Params{ty_alias<ty_access<ty_alias<ty_bool_>>>, ty_access<ty_bool_>},
+    Params{ty_alias<ty_access<ty_alias<ty_vec3<ty_access<ty_f32>>>>>,
+           ty_access<ty_vec3<ty_access<ty_f32>>>},
+    Params{ty_alias<ty_access<ty_alias<ty_mat3x3<ty_access<ty_f32>>>>>,
+           ty_access<ty_mat3x3<ty_access<ty_f32>>>},
 };
 
 using CanonicalTest = ResolverTestWithParam<Params>;