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/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) {
