tint: Improve module scope var diagnostic
Improve the error message when declaring a module-scope var without an address space, with an initializer.
Fixed: tint:1870
Change-Id: If087ae7dadb512c7050e89a0c75990080668e27d
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/123600
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
diff --git a/src/tint/resolver/resolver.cc b/src/tint/resolver/resolver.cc
index 597821f..74b32bb 100644
--- a/src/tint/resolver/resolver.cc
+++ b/src/tint/resolver/resolver.cc
@@ -260,7 +260,7 @@
ty = rhs->Type()->UnwrapRef(); // Implicit load of RHS
}
- if (rhs && !validator_.VariableInitializer(v, builtin::AddressSpace::kUndefined, ty, rhs)) {
+ if (rhs && !validator_.VariableInitializer(v, ty, rhs)) {
return nullptr;
}
@@ -323,7 +323,7 @@
return nullptr;
}
- if (rhs && !validator_.VariableInitializer(v, builtin::AddressSpace::kUndefined, ty, rhs)) {
+ if (rhs && !validator_.VariableInitializer(v, ty, rhs)) {
return nullptr;
}
@@ -417,7 +417,7 @@
ty = rhs->Type();
}
- if (!validator_.VariableInitializer(c, builtin::AddressSpace::kUndefined, ty, rhs)) {
+ if (!validator_.VariableInitializer(c, ty, rhs)) {
return nullptr;
}
@@ -516,7 +516,7 @@
access = DefaultAccessForAddressSpace(address_space);
}
- if (rhs && !validator_.VariableInitializer(var, address_space, storage_ty, rhs)) {
+ if (rhs && !validator_.VariableInitializer(var, storage_ty, rhs)) {
return nullptr;
}
diff --git a/src/tint/resolver/validator.cc b/src/tint/resolver/validator.cc
index f8f4c4d..c3b8a3d 100644
--- a/src/tint/resolver/validator.cc
+++ b/src/tint/resolver/validator.cc
@@ -373,7 +373,6 @@
}
bool Validator::VariableInitializer(const ast::Variable* v,
- builtin::AddressSpace address_space,
const type::Type* storage_ty,
const sem::ValueExpression* initializer) const {
auto* initializer_ty = initializer->Type();
@@ -388,23 +387,6 @@
return false;
}
- if (v->Is<ast::Var>()) {
- switch (address_space) {
- case builtin::AddressSpace::kPrivate:
- case builtin::AddressSpace::kFunction:
- break; // Allowed an initializer
- default:
- // https://gpuweb.github.io/gpuweb/wgsl/#var-and-let
- // Optionally has an initializer expression, if the variable is in the private or
- // function address spaces.
- AddError("var of address space '" + utils::ToString(address_space) +
- "' cannot have an initializer. var initializers are only supported "
- "for the address spaces 'private' and 'function'",
- v->source);
- return false;
- }
- }
-
return true;
}
@@ -728,6 +710,23 @@
}
}
+ if (var->initializer) {
+ switch (v->AddressSpace()) {
+ case builtin::AddressSpace::kPrivate:
+ case builtin::AddressSpace::kFunction:
+ break; // Allowed an initializer
+ default:
+ // https://gpuweb.github.io/gpuweb/wgsl/#var-and-let
+ // Optionally has an initializer expression, if the variable is in the private or
+ // function address spaces.
+ AddError("var of address space '" + utils::ToString(v->AddressSpace()) +
+ "' cannot have an initializer. var initializers are only supported "
+ "for the address spaces 'private' and 'function'",
+ var->source);
+ return false;
+ }
+ }
+
if (!CheckTypeAccessAddressSpace(v->Type()->UnwrapRef(), v->Access(), v->AddressSpace(),
var->attributes, var->source)) {
return false;
diff --git a/src/tint/resolver/validator.h b/src/tint/resolver/validator.h
index 45c98c8..e0e3051 100644
--- a/src/tint/resolver/validator.h
+++ b/src/tint/resolver/validator.h
@@ -429,12 +429,10 @@
/// Validates a variable initializer
/// @param v the variable to validate
- /// @param address_space the address space of the variable
/// @param storage_type the type of the storage
/// @param initializer the RHS initializer expression
/// @returns true on succes, false otherwise
bool VariableInitializer(const ast::Variable* v,
- builtin::AddressSpace address_space,
const type::Type* storage_type,
const sem::ValueExpression* initializer) const;
diff --git a/src/tint/resolver/variable_validation_test.cc b/src/tint/resolver/variable_validation_test.cc
index 6ca3a08..7f62e8e 100644
--- a/src/tint/resolver/variable_validation_test.cc
+++ b/src/tint/resolver/variable_validation_test.cc
@@ -58,6 +58,26 @@
EXPECT_EQ(r()->error(), "12:34 error: builtin 'storageBarrier' does not return a value");
}
+TEST_F(ResolverVariableValidationTest, GlobalVarNoAddressSpace) {
+ // var a : i32;
+ GlobalVar(Source{{12, 34}}, "a", ty.i32());
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(
+ r()->error(),
+ R"(12:34 error: module-scope 'var' declarations that are not of texture or sampler types must provide an address space)");
+}
+
+TEST_F(ResolverVariableValidationTest, GlobalVarWithInitializerNoAddressSpace) {
+ // var a = 1;
+ GlobalVar(Source{{12, 34}}, "a", Expr(1_a));
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(
+ r()->error(),
+ R"(12:34 error: module-scope 'var' declarations that are not of texture or sampler types must provide an address space)");
+}
+
TEST_F(ResolverVariableValidationTest, GlobalVarUsedAtModuleScope) {
// var<private> a : i32;
// var<private> b : i32 = a;
