validation: atomics access mode and storage class

Bug: tint:901 tint:909
Change-Id: Ibbcdd80ddbe2aa9940bbd73bb404349afc633836
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/60080
Auto-Submit: Sarah Mashayekhi <sarahmashay@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Sarah Mashayekhi <sarahmashay@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>

src/resolver/atomics_validation_test.cc

@@ -26,7 +26,23 @@ namespace {
 struct ResolverAtomicValidationTest : public resolver::TestHelper,
                                       public testing::Test {};
 
-TEST_F(ResolverAtomicValidationTest, GlobalOfInvalidType) {
+TEST_F(ResolverAtomicValidationTest, StorageClass_WorkGroup) {
+  Global("a", ty.atomic(Source{{12, 34}}, ty.i32()),
+         ast::StorageClass::kWorkgroup);
+
+  EXPECT_TRUE(r()->Resolve());
+}
+
+TEST_F(ResolverAtomicValidationTest, StorageClass_Storage) {
+  auto* s = Structure("s", {Member("a", ty.atomic(Source{{12, 34}}, ty.i32()))},
+                      {StructBlock()});
+  Global("g", ty.Of(s), ast::StorageClass::kStorage, ast::Access::kReadWrite,
+         GroupAndBinding(0, 0));
+
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+}
+
+TEST_F(ResolverAtomicValidationTest, InvalidType) {
   Global("a", ty.atomic(ty.f32(Source{{12, 34}})),
          ast::StorageClass::kWorkgroup);
 
@@ -34,28 +50,274 @@ TEST_F(ResolverAtomicValidationTest, GlobalOfInvalidType) {
   EXPECT_EQ(r()->error(), "12:34 error: atomic only supports i32 or u32 types");
 }
 
-// TODO(crbug.com/tint/909): add validation and enable this test
+TEST_F(ResolverAtomicValidationTest, InvalidStorageClass_Simple) {
-TEST_F(ResolverAtomicValidationTest, DISABLED_GlobalOfInvalidStorageClass) {
   Global("a", ty.atomic(Source{{12, 34}}, ty.i32()),
          ast::StorageClass::kPrivate);
 
   EXPECT_FALSE(r()->Resolve());
-  EXPECT_EQ(r()->error(), "12:34 error: atomic var requires workgroup storage");
+  EXPECT_EQ(r()->error(),
+            "12:34 error: atomic variables must have <storage> or <workgroup> "
+            "storage class");
 }
 
-TEST_F(ResolverAtomicValidationTest, GlobalPrivateStruct) {
+TEST_F(ResolverAtomicValidationTest, InvalidStorageClass_Array) {
+  Global("a", ty.atomic(Source{{12, 34}}, ty.i32()),
+         ast::StorageClass::kPrivate);
+
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(r()->error(),
+            "12:34 error: atomic variables must have <storage> or <workgroup> "
+            "storage class");
+}
+
+TEST_F(ResolverAtomicValidationTest, InvalidStorageClass_Struct) {
   auto* s =
       Structure("s", {Member("a", ty.atomic(Source{{12, 34}}, ty.i32()))});
   Global("g", ty.Of(s), ast::StorageClass::kPrivate);
 
   EXPECT_FALSE(r()->Resolve());
   EXPECT_EQ(r()->error(),
-            "12:34 error: atomic types can only be used in storage classes "
+            "error: atomic variables must have <storage> or <workgroup> "
-            "workgroup or storage, but was used by storage class private");
+            "storage class\n"
+            "note: atomic sub-type of 's' is declared here");
 }
 
-// TODO(crbug.com/tint/901): Validate that storage buffer access mode is
+TEST_F(ResolverAtomicValidationTest, InvalidStorageClass_StructOfStruct) {
-// read_write.
+  // struct Inner { m : atomic<i32>; };
+  // struct Outer { m : array<Inner, 4>; };
+  // var<private> g : Outer;
+
+  auto* Inner =
+      Structure("Inner", {Member("m", ty.atomic(Source{{12, 34}}, ty.i32()))});
+  auto* Outer = Structure("Outer", {Member("m", ty.Of(Inner))});
+  Global("g", ty.Of(Outer), ast::StorageClass::kPrivate);
+
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(r()->error(),
+            "error: atomic variables must have <storage> or <workgroup> "
+            "storage class\n"
+            "note: atomic sub-type of 'Outer' is declared here");
+}
+
+TEST_F(ResolverAtomicValidationTest,
+       InvalidStorageClass_StructOfStructOfArray) {
+  // struct Inner { m : array<atomic<i32>, 4>; };
+  // struct Outer { m : array<Inner, 4>; };
+  // var<private> g : Outer;
+
+  auto* Inner =
+      Structure("Inner", {Member(Source{{12, 34}}, "m", ty.atomic(ty.i32()))});
+  auto* Outer = Structure("Outer", {Member("m", ty.Of(Inner))});
+  Global("g", ty.Of(Outer), ast::StorageClass::kPrivate);
+
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(r()->error(),
+            "error: atomic variables must have <storage> or <workgroup> "
+            "storage class\n"
+            "12:34 note: atomic sub-type of 'Outer' is declared here");
+}
+
+TEST_F(ResolverAtomicValidationTest, InvalidStorageClass_ArrayOfArray) {
+  // type AtomicArray = array<atomic<i32>, 5>;
+  // var<private> v: array<s, 5>;
+
+  auto* atomic_array = Alias(Source{{12, 34}}, "AtomicArray",
+                             ty.atomic(Source{{12, 34}}, ty.i32()));
+  Global(Source{{56, 78}}, "v", ty.Of(atomic_array),
+         ast::StorageClass::kPrivate);
+
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(r()->error(),
+            "error: atomic variables must have <storage> or <workgroup> "
+            "storage class");
+}
+
+TEST_F(ResolverAtomicValidationTest, InvalidStorageClass_ArrayOfStruct) {
+  // struct S{
+  //   m: atomic<u32>;
+  // };
+  // var<private> v: array<S, 5>;
+
+  auto* s = Structure("S", {Member("m", ty.atomic<u32>())});
+  Global(Source{{56, 78}}, "v", ty.array(ty.Of(s), 5),
+         ast::StorageClass::kPrivate);
+
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(r()->error(),
+            "error: atomic variables must have <storage> or <workgroup> "
+            "storage class\n"
+            "note: atomic sub-type of 'array<S, 5>' is declared here");
+}
+
+TEST_F(ResolverAtomicValidationTest, InvalidStorageClass_ArrayOfStructOfArray) {
+  // type AtomicArray = array<atomic<i32>, 5>;
+  // struct S{
+  //   m: AtomicArray;
+  // };
+  // var<private> v: array<S, 5>;
+
+  auto* atomic_array = Alias(Source{{12, 34}}, "AtomicArray",
+                             ty.atomic(Source{{12, 34}}, ty.i32()));
+  auto* s = Structure("S", {Member("m", ty.Of(atomic_array))});
+  Global(Source{{56, 78}}, "v", ty.array(ty.Of(s), 5),
+         ast::StorageClass::kPrivate);
+
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(r()->error(),
+            "error: atomic variables must have <storage> or <workgroup> "
+            "storage class\n"
+            "note: atomic sub-type of 'array<S, 5>' is declared here");
+}
+
+TEST_F(ResolverAtomicValidationTest, InvalidStorageClass_Complex) {
+  // type AtomicArray = array<atomic<i32>, 5>;
+  // struct S6 { x: array<i32, 4>; };
+  // struct S5 { x: S6;
+  //             y: AtomicArray;
+  //             z: array<atomic<u32>, 8>; };
+  // struct S4 { x: S6;
+  //             y: S5;
+  //             z: array<atomic<i32>, 4>; };
+  // struct S3 { x: S4; };
+  // struct S2 { x: S3; };
+  // struct S1 { x: S2; };
+  // struct S0 { x: S1; };
+  // var<private> g : S0;
+
+  auto* atomic_array = Alias(Source{{12, 34}}, "AtomicArray",
+                             ty.atomic(Source{{12, 34}}, ty.i32()));
+  auto* array_i32_4 = ty.array(ty.i32(), 4);
+  auto* array_atomic_u32_8 = ty.array(ty.atomic(ty.u32()), 8);
+  auto* array_atomic_i32_4 = ty.array(ty.atomic(ty.i32()), 4);
+
+  auto* s6 = Structure("S6", {Member("x", array_i32_4)});
+  auto* s5 = Structure("S5", {Member("x", ty.Of(s6)),             //
+                              Member("y", ty.Of(atomic_array)),   //
+                              Member("z", array_atomic_u32_8)});  //
+  auto* s4 = Structure("S4", {Member("x", ty.Of(s6)),             //
+                              Member("y", ty.Of(s5)),             //
+                              Member("z", array_atomic_i32_4)});  //
+  auto* s3 = Structure("S3", {Member("x", ty.Of(s4))});
+  auto* s2 = Structure("S2", {Member("x", ty.Of(s3))});
+  auto* s1 = Structure("S1", {Member("x", ty.Of(s2))});
+  auto* s0 = Structure("S0", {Member("x", ty.Of(s1))});
+  Global(Source{{56, 78}}, "g", ty.Of(s0), ast::StorageClass::kPrivate);
+
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(r()->error(),
+            "error: atomic variables must have <storage> or <workgroup> "
+            "storage class\n"
+            "note: atomic sub-type of 'S0' is declared here");
+}
+
+TEST_F(ResolverAtomicValidationTest, Struct_AccessMode_Read) {
+  auto* s = Structure("s", {Member("a", ty.atomic(Source{{12, 34}}, ty.i32()))},
+                      {StructBlock()});
+  Global(Source{{56, 78}}, "g", ty.Of(s), ast::StorageClass::kStorage,
+         ast::Access::kRead, GroupAndBinding(0, 0));
+
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(
+      r()->error(),
+      "error: atomic variables in <storage> storage class must have read_write "
+      "access mode\n"
+      "note: atomic sub-type of 's' is declared here");
+}
+
+TEST_F(ResolverAtomicValidationTest, InvalidAccessMode_Struct) {
+  auto* s = Structure("s", {Member("a", ty.atomic(Source{{12, 34}}, ty.i32()))},
+                      {StructBlock()});
+  Global(Source{{56, 78}}, "g", ty.Of(s), ast::StorageClass::kStorage,
+         ast::Access::kRead, GroupAndBinding(0, 0));
+
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(
+      r()->error(),
+      "error: atomic variables in <storage> storage class must have read_write "
+      "access mode\n"
+      "note: atomic sub-type of 's' is declared here");
+}
+
+TEST_F(ResolverAtomicValidationTest, InvalidAccessMode_StructOfStruct) {
+  // struct Inner { m : atomic<i32>; };
+  // struct Outer { m : array<Inner, 4>; };
+  // var<storage, read> g : Outer;
+
+  auto* Inner =
+      Structure("Inner", {Member("m", ty.atomic(Source{{12, 34}}, ty.i32()))});
+  auto* Outer =
+      Structure("Outer", {Member("m", ty.Of(Inner))}, {StructBlock()});
+  Global(Source{{56, 78}}, "g", ty.Of(Outer), ast::StorageClass::kStorage,
+         ast::Access::kRead, GroupAndBinding(0, 0));
+
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(
+      r()->error(),
+      "error: atomic variables in <storage> storage class must have read_write "
+      "access mode\n"
+      "note: atomic sub-type of 'Outer' is declared here");
+}
+
+TEST_F(ResolverAtomicValidationTest, InvalidAccessMode_StructOfStructOfArray) {
+  // struct Inner { m : array<atomic<i32>, 4>; };
+  // struct Outer { m : array<Inner, 4>; };
+  // var<storage, read> g : Outer;
+
+  auto* Inner =
+      Structure("Inner", {Member(Source{{12, 34}}, "m", ty.atomic(ty.i32()))});
+  auto* Outer =
+      Structure("Outer", {Member("m", ty.Of(Inner))}, {StructBlock()});
+  Global(Source{{56, 78}}, "g", ty.Of(Outer), ast::StorageClass::kStorage,
+         ast::Access::kRead, GroupAndBinding(0, 0));
+
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(r()->error(),
+            "error: atomic variables in <storage> storage class must have "
+            "read_write access mode\n"
+            "12:34 note: atomic sub-type of 'Outer' is declared here");
+}
+
+TEST_F(ResolverAtomicValidationTest, InvalidAccessMode_Complex) {
+  // type AtomicArray = array<atomic<i32>, 5>;
+  // struct S6 { x: array<i32, 4>; };
+  // struct S5 { x: S6;
+  //             y: AtomicArray;
+  //             z: array<atomic<u32>, 8>; };
+  // struct S4 { x: S6;
+  //             y: S5;
+  //             z: array<atomic<i32>, 4>; };
+  // struct S3 { x: S4; };
+  // struct S2 { x: S3; };
+  // struct S1 { x: S2; };
+  // struct S0 { x: S1; };
+  // var<storage, read> g : S0;
+
+  auto* atomic_array = Alias(Source{{12, 34}}, "AtomicArray",
+                             ty.atomic(Source{{12, 34}}, ty.i32()));
+  auto* array_i32_4 = ty.array(ty.i32(), 4);
+  auto* array_atomic_u32_8 = ty.array(ty.atomic(ty.u32()), 8);
+  auto* array_atomic_i32_4 = ty.array(ty.atomic(ty.i32()), 4);
+
+  auto* s6 = Structure("S6", {Member("x", array_i32_4)});
+  auto* s5 = Structure("S5", {Member("x", ty.Of(s6)),             //
+                              Member("y", ty.Of(atomic_array)),   //
+                              Member("z", array_atomic_u32_8)});  //
+  auto* s4 = Structure("S4", {Member("x", ty.Of(s6)),             //
+                              Member("y", ty.Of(s5)),             //
+                              Member("z", array_atomic_i32_4)});  //
+  auto* s3 = Structure("S3", {Member("x", ty.Of(s4))});
+  auto* s2 = Structure("S2", {Member("x", ty.Of(s3))});
+  auto* s1 = Structure("S1", {Member("x", ty.Of(s2))});
+  auto* s0 = Structure("S0", {Member("x", ty.Of(s1))}, {StructBlock()});
+  Global(Source{{56, 78}}, "g", ty.Of(s0), ast::StorageClass::kStorage,
+         ast::Access::kRead, GroupAndBinding(0, 0));
+
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(r()->error(),
+            "error: atomic variables in <storage> storage class must have "
+            "read_write access mode\n"
+            "note: atomic sub-type of 'S0' is declared here");
+}
 
 TEST_F(ResolverAtomicValidationTest, Local) {
   WrapInFunction(Var("a", ty.atomic(Source{{12, 34}}, ty.i32())));

src/resolver/resolver.cc

@@ -265,12 +265,8 @@ bool Resolver::ResolveInternal() {
   if (!ValidatePipelineStages()) {
     return false;
   }
-  if (!ValidateAtomicUses()) {
-    return false;
-  }
 
   bool result = true;
-
   for (auto* node : builder_->ASTNodes().Objects()) {
     if (marked_.count(node) == 0) {
       TINT_ICE(Resolver, diagnostics_)
@@ -421,34 +417,6 @@ bool Resolver::ValidateAtomic(const ast::Atomic* a, const sem::Atomic* s) {
   return true;
 }
 
-bool Resolver::ValidateAtomicUses() {
-  // https://gpuweb.github.io/gpuweb/wgsl/#atomic-types
-  // Atomic types may only be instantiated by variables in the workgroup storage
-  // class or by storage buffer variables with a read_write access mode.
-  for (auto sm : atomic_members_) {
-    auto* structure = sm.structure;
-    for (auto usage : structure->StorageClassUsage()) {
-      if (usage == ast::StorageClass::kWorkgroup) {
-        continue;
-      }
-      if (usage != ast::StorageClass::kStorage) {
-        // TODO(crbug.com/tint/901): Validate that the access mode is
-        // read_write.
-        auto* member = structure->Members()[sm.index];
-        AddError(
-            "atomic types can only be used in storage classes workgroup or "
-            "storage, but was used by storage class " +
-                std::string(ast::str(usage)),
-            member->Declaration()->type()->source());
-        // TODO(crbug.com/tint/901): Add note pointing at where the usage came
-        // from.
-        return false;
-      }
-    }
-  }
-  return true;
-}
-
 bool Resolver::ValidateStorageTexture(const ast::StorageTexture* t) {
   switch (t->access()) {
     case ast::Access::kUndefined:
@@ -979,8 +947,7 @@ bool Resolver::ValidateGlobalVariable(const VariableInfo* info) {
   if (info->storage_class != ast::StorageClass::kStorage &&
       info->declaration->declared_access() != ast::Access::kUndefined) {
     AddError(
-        "variables not in <storage> storage class must not declare an access "
+        "only variables in <storage> storage class may declare an access mode",
-        "mode",
         info->declaration->source());
     return false;
   }
@@ -1060,9 +1027,62 @@ bool Resolver::ValidateGlobalVariable(const VariableInfo* info) {
       break;
   }
 
+  if (!info->declaration->is_const()) {
+    if (!ValidateAtomicVariable(info)) {
+      return false;
+    }
+  }
+
   return ValidateVariable(info);
 }
 
+// https://gpuweb.github.io/gpuweb/wgsl/#atomic-types
+// Atomic types may only be instantiated by variables in the workgroup storage
+// class or by storage buffer variables with a read_write access mode.
+bool Resolver::ValidateAtomicVariable(const VariableInfo* info) {
+  auto sc = info->storage_class;
+  auto access = info->access;
+  auto* type = info->type->UnwrapRef();
+  auto source = info->declaration->type()->source();
+
+  if (type->Is<sem::Atomic>()) {
+    if (sc != ast::StorageClass::kWorkgroup) {
+      AddError(
+          "atomic variables must have <storage> or <workgroup> storage class",
+          info->declaration->type()->source());
+      return false;
+    }
+  } else if (type->IsAnyOf<sem::Struct, sem::Array>()) {
+    auto found = atomic_composite_info_.find(type);
+    if (found != atomic_composite_info_.end()) {
+      if (sc != ast::StorageClass::kStorage &&
+          sc != ast::StorageClass::kWorkgroup) {
+        AddError(
+            "atomic variables must have <storage> or <workgroup> storage class",
+            source);
+        AddNote("atomic sub-type of '" +
+                    type->FriendlyName(builder_->Symbols()) +
+                    "' is declared here",
+                found->second);
+        return false;
+      } else if (sc == ast::StorageClass::kStorage &&
+                 access != ast::Access::kReadWrite) {
+        AddError(
+            "atomic variables in <storage> storage class must have read_write "
+            "access mode",
+            source);
+        AddNote("atomic sub-type of '" +
+                    type->FriendlyName(builder_->Symbols()) +
+                    "' is declared here",
+                found->second);
+        return false;
+      }
+    }
+  }
+
+  return true;
+}
+
 bool Resolver::ValidateVariable(const VariableInfo* info) {
   auto* var = info->declaration;
   auto* storage_type = info->type->UnwrapRef();
@@ -3738,20 +3758,20 @@ void Resolver::CreateSemanticNodes() const {
 sem::Array* Resolver::Array(const ast::Array* arr) {
   auto source = arr->source();
 
-  auto* el_ty = Type(arr->type());
+  auto* elem_type = Type(arr->type());
-  if (!el_ty) {
+  if (!elem_type) {
     return nullptr;
   }
 
-  if (!IsPlain(el_ty)) {  // Check must come before GetDefaultAlignAndSize()
+  if (!IsPlain(elem_type)) {  // Check must come before GetDefaultAlignAndSize()
-    AddError(el_ty->FriendlyName(builder_->Symbols()) +
+    AddError(elem_type->FriendlyName(builder_->Symbols()) +
                  " cannot be used as an element type of an array",
              source);
     return nullptr;
   }
 
-  uint32_t el_align = el_ty->Align();
+  uint32_t el_align = elem_type->Align();
-  uint32_t el_size = el_ty->Size();
+  uint32_t el_size = elem_type->Size();
 
   if (!ValidateNoDuplicateDecorations(arr->decorations())) {
     return nullptr;
@@ -3781,14 +3801,23 @@ sem::Array* Resolver::Array(const ast::Array* arr) {
   // WebGPU requires runtime arrays have at least one element, but the AST
   // records an element count of 0 for it.
   auto size = std::max<uint32_t>(arr->size(), 1) * stride;
-  auto* sem = builder_->create<sem::Array>(el_ty, arr->size(), el_align, size,
+  auto* out = builder_->create<sem::Array>(elem_type, arr->size(), el_align,
-                                           stride, implicit_stride);
+                                           size, stride, implicit_stride);
 
-  if (!ValidateArray(sem, source)) {
+  if (!ValidateArray(out, source)) {
     return nullptr;
   }
 
-  return sem;
+  if (elem_type->Is<sem::Atomic>()) {
+    atomic_composite_info_.emplace(out, arr->type()->source());
+  } else {
+    auto found = atomic_composite_info_.find(elem_type);
+    if (found != atomic_composite_info_.end()) {
+      atomic_composite_info_.emplace(out, found->second);
+    }
+  }
+
+  return out;
 }
 
 bool Resolver::ValidateArray(const sem::Array* arr, const Source& source) {
@@ -4090,11 +4119,18 @@ sem::Struct* Resolver::Structure(const ast::Struct* str) {
       builder_->create<sem::Struct>(str, str->name(), sem_members, struct_align,
                                     struct_size, size_no_padding);
 
-  // Keep track of atomic members for validation after all usages have been
-  // determined.
   for (size_t i = 0; i < sem_members.size(); i++) {
-    if (sem_members[i]->Type()->Is<sem::Atomic>()) {
+    auto* mem_type = sem_members[i]->Type();
-      atomic_members_.emplace_back(StructMember{out, i});
+    if (mem_type->Is<sem::Atomic>()) {
+      atomic_composite_info_.emplace(out,
+                                     sem_members[i]->Declaration()->source());
+      break;
+    } else {
+      auto found = atomic_composite_info_.find(mem_type);
+      if (found != atomic_composite_info_.end()) {
+        atomic_composite_info_.emplace(out, found->second);
+        break;
+      }
     }
   }
 

src/resolver/resolver.h

@@ -207,13 +207,6 @@ class Resolver {
     sem::Type* const sem;
   };
 
-  // Structure holding a pointer to the sem::Struct and an index to a member of
-  // that structure.
-  struct StructMember {
-    sem::Struct* structure;
-    size_t index;
-  };
-
   /// Resolves the program, without creating final the semantic nodes.
   /// @returns true on success, false on error
   bool ResolveInternal();
@@ -277,7 +270,7 @@ class Resolver {
                                      uint32_t el_align,
                                      const Source& source);
   bool ValidateAtomic(const ast::Atomic* a, const sem::Atomic* s);
-  bool ValidateAtomicUses();
+  bool ValidateAtomicVariable(const VariableInfo* info);
   bool ValidateAssignment(const ast::AssignmentStatement* a);
   bool ValidateBuiltinDecoration(const ast::BuiltinDecoration* deco,
                                  const sem::Type* storage_type,
@@ -470,7 +463,7 @@ class Resolver {
   ScopeStack<VariableInfo*> variable_stack_;
   std::unordered_map<Symbol, FunctionInfo*> symbol_to_function_;
   std::vector<FunctionInfo*> entry_points_;
-  std::vector<StructMember> atomic_members_;
+  std::unordered_map<const sem::Type*, const Source&> atomic_composite_info_;
   std::unordered_map<const ast::Function*, FunctionInfo*> function_to_info_;
   std::unordered_map<const ast::Variable*, VariableInfo*> variable_to_info_;
   std::unordered_map<const ast::CallExpression*, FunctionCallInfo>

src/resolver/storage_class_validation_test.cc

@@ -108,7 +108,7 @@ TEST_F(ResolverStorageClassValidationTest, NotStorage_AccessMode) {
 
   EXPECT_EQ(
       r()->error(),
-      R"(56:78 error: variables not in <storage> storage class must not declare an access mode)");
+      R"(56:78 error: only variables in <storage> storage class may declare an access mode)");
 }
 
 TEST_F(ResolverStorageClassValidationTest, StorageBufferNoBlockDecoration) {