resolver: Storable types include textures/samplers
Add a Resolver::IsPlain() method to check for plain types, which is
then used instead of IsStorable() for validating array and struct
subtypes.
Remove validation of assignment and constructor RHS types, instead
validating the type of the variable declaration. This catches
additional errors that were previously missed, such as using a pointer
for a var declaration with no constructor.
Change-Id: I5786a262159d2a42cc05b44743c6c26f6b5647c0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53960
Auto-Submit: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
diff --git a/src/ast/module_clone_test.cc b/src/ast/module_clone_test.cc
index 37a1129..e635d56 100644
--- a/src/ast/module_clone_test.cc
+++ b/src/ast/module_clone_test.cc
@@ -66,7 +66,7 @@
var l3 : vec2<u32> = vec2<u32>(u32(l0), u32(l1));
var l4 : S;
var l5 : u32 = l4.m1[5];
- var l6 : ptr<private, u32>;
+ let l6 : ptr<private, u32> = &g0;
loop {
l0 = (p1 + 2);
if (((l0 % 4) == 0)) {
diff --git a/src/resolver/assignment_validation_test.cc b/src/resolver/assignment_validation_test.cc
index b908662..b270ec9 100644
--- a/src/resolver/assignment_validation_test.cc
+++ b/src/resolver/assignment_validation_test.cc
@@ -155,7 +155,10 @@
EXPECT_EQ(r()->error(), "12:34 error: cannot assign to value of type 'i32'");
}
-TEST_F(ResolverAssignmentValidationTest, AssignNonStorable_Fail) {
+// TODO(crbug.com/tint/809): The var has an implicit access::read, and so this
+// test will pass again when we start validating the access mode on the LHS of
+// an assignment.
+TEST_F(ResolverAssignmentValidationTest, DISABLED_AssignNonStorable_Fail) {
// var a : texture_storage_1d<rgba8unorm, read>;
// var b : texture_storage_1d<rgba8unorm, read>;
// a = b;
diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index ff4f455..fc25d8f 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -165,20 +165,18 @@
return result;
}
-// https://gpuweb.github.io/gpuweb/wgsl.html#storable-types
-bool Resolver::IsStorable(const sem::Type* type) {
- if (type->is_scalar() || type->Is<sem::Vector>() || type->Is<sem::Matrix>()) {
+// https://gpuweb.github.io/gpuweb/wgsl/#plain-types-section
+bool Resolver::IsPlain(const sem::Type* type) {
+ if (type->is_scalar() || type->Is<sem::Vector>() || type->Is<sem::Matrix>() ||
+ type->Is<sem::Array>() || type->Is<sem::Struct>()) {
return true;
}
- if (auto* arr = type->As<sem::Array>()) {
- return IsStorable(arr->ElemType());
- }
- if (auto* str = type->As<sem::Struct>()) {
- for (const auto* member : str->Members()) {
- if (!IsStorable(member->Type())) {
- return false;
- }
- }
+ return false;
+}
+
+// https://gpuweb.github.io/gpuweb/wgsl.html#storable-types
+bool Resolver::IsStorable(const sem::Type* type) {
+ if (IsPlain(type) || type->Is<sem::Texture>() || type->Is<sem::Sampler>()) {
return true;
}
return false;
@@ -525,14 +523,6 @@
const std::string& rhs_type_name) {
auto* value_type = rhs_type->UnwrapRef(); // Implicit load of RHS
- // RHS needs to be of a storable type
- if (!var->is_const() && !IsStorable(value_type)) {
- diagnostics_.add_error(
- "'" + rhs_type_name + "' is not storable for assignment",
- var->constructor()->source());
- return false;
- }
-
// Value type has to match storage type
if (storage_type != value_type) {
std::string decl = var->is_const() ? "let" : "var";
@@ -769,6 +759,14 @@
bool Resolver::ValidateVariable(const VariableInfo* info) {
auto* var = info->declaration;
auto* storage_type = info->type->UnwrapRef();
+
+ if (!var->is_const() && !IsStorable(storage_type)) {
+ diagnostics_.add_error(storage_type->FriendlyName(builder_->Symbols()) +
+ " cannot be used as the type of a var",
+ var->source());
+ return false;
+ }
+
if (auto* r = storage_type->As<sem::Array>()) {
if (r->IsRuntimeSized()) {
diagnostics_.add_error(
@@ -2750,7 +2748,7 @@
return nullptr;
}
- if (!IsStorable(el_ty)) { // Check must come before DefaultAlignAndSize()
+ if (!IsPlain(el_ty)) { // Check must come before DefaultAlignAndSize()
builder_->Diagnostics().add_error(
el_ty->FriendlyName(builder_->Symbols()) +
" cannot be used as an element type of an array",
@@ -2924,7 +2922,7 @@
}
// Validate member type
- if (!IsStorable(type)) {
+ if (!IsPlain(type)) {
diagnostics_.add_error(
type->FriendlyName(builder_->Symbols()) +
" cannot be used as the type of a structure member");
@@ -3167,13 +3165,6 @@
auto* value_type = rhs_type->UnwrapRef(); // Implicit load of RHS
- // RHS needs to be of a storable type
- if (!IsStorable(value_type)) {
- diagnostics_.add_error("'" + TypeNameOf(a->rhs()) + "' is not storable",
- a->rhs()->source());
- return false;
- }
-
// Value type has to match storage type
if (storage_type != value_type) {
diagnostics_.add_error("cannot assign '" + TypeNameOf(a->rhs()) + "' to '" +
diff --git a/src/resolver/resolver.h b/src/resolver/resolver.h
index da5ba4b..da9a0a4 100644
--- a/src/resolver/resolver.h
+++ b/src/resolver/resolver.h
@@ -74,6 +74,10 @@
bool Resolve();
/// @param type the given type
+ /// @returns true if the given type is a plain type
+ bool IsPlain(const sem::Type* type);
+
+ /// @param type the given type
/// @returns true if the given type is storable
bool IsStorable(const sem::Type* type);
diff --git a/src/resolver/var_let_validation_test.cc b/src/resolver/var_let_validation_test.cc
index 7e5ecf2..96ada20 100644
--- a/src/resolver/var_let_validation_test.cc
+++ b/src/resolver/var_let_validation_test.cc
@@ -43,18 +43,19 @@
"12:34 error: let declarations must have initializers");
}
-TEST_F(ResolverVarLetValidationTest, VarConstructorNotStorable) {
+TEST_F(ResolverVarLetValidationTest, VarTypeNotStorable) {
// var i : i32;
// var p : pointer<function, i32> = &v;
auto* i = Var("i", ty.i32(), ast::StorageClass::kNone);
- auto* p = Var("a", ty.i32(), ast::StorageClass::kNone,
- AddressOf(Source{{12, 34}}, "i"));
+ auto* p =
+ Var(Source{{56, 78}}, "a", ty.pointer<i32>(ast::StorageClass::kFunction),
+ ast::StorageClass::kNone, AddressOf(Source{{12, 34}}, "i"));
WrapInFunction(i, p);
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
- "12:34 error: 'ptr<function, i32, read_write>' is not storable for "
- "assignment");
+ "56:78 error: ptr<function, i32, read_write> cannot be used as the "
+ "type of a var");
}
TEST_F(ResolverVarLetValidationTest, LetConstructorWrongType) {
