tint/resolver: Shuffle validation code
Restructure the logic so there's less, pointless dynamic casting, and the complexity is reduced.
This alters the order in which variables are validated, hence the change of test.
Change-Id: I9a3120c0278faa5ac9f1db65eeb71a8e4a705596
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/95948
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
diff --git a/src/tint/program_builder.h b/src/tint/program_builder.h
index e9b0a80..9a4f7ca 100644
--- a/src/tint/program_builder.h
+++ b/src/tint/program_builder.h
@@ -780,6 +780,17 @@
}
/// @param source the Source of the node
+ /// @param storage_class the storage class of the pointer
+ /// @param access the optional access control of the pointer
+ /// @return the pointer to type `T` with the given ast::StorageClass.
+ template <typename T>
+ const ast::Pointer* pointer(const Source& source,
+ ast::StorageClass storage_class,
+ ast::Access access = ast::Access::kUndefined) const {
+ return pointer(source, Of<T>(), storage_class, access);
+ }
+
+ /// @param source the Source of the node
/// @param type the type of the atomic
/// @return the atomic to `type`
const ast::Atomic* atomic(const Source& source, const ast::Type* type) const {
diff --git a/src/tint/resolver/resolver.cc b/src/tint/resolver/resolver.cc
index 12fca5a..0df9a8f 100644
--- a/src/tint/resolver/resolver.cc
+++ b/src/tint/resolver/resolver.cc
@@ -2575,7 +2575,7 @@
sem->Behaviors() = ctor->Behaviors();
}
- return validator_.Variable(variable);
+ return validator_.LocalVariable(variable);
});
}
diff --git a/src/tint/resolver/validator.cc b/src/tint/resolver/validator.cc
index 121cc11..c7afb54 100644
--- a/src/tint/resolver/validator.cc
+++ b/src/tint/resolver/validator.cc
@@ -533,6 +533,32 @@
return true;
}
+bool Validator::LocalVariable(const sem::Variable* v) const {
+ auto* decl = v->Declaration();
+ return Switch(
+ decl, //
+ [&](const ast::Var* var) {
+ if (IsValidationEnabled(var->attributes,
+ ast::DisabledValidation::kIgnoreStorageClass)) {
+ if (!v->Type()->UnwrapRef()->IsConstructible()) {
+ AddError("function-scope 'var' must have a constructible type",
+ var->type ? var->type->source : var->source);
+ return false;
+ }
+ }
+ return Var(v);
+ }, //
+ [&](const ast::Let*) { return Let(v); }, //
+ [&](const ast::Override*) { return Override(v); }, //
+ [&](const ast::Const*) { return true; }, //
+ [&](Default) {
+ TINT_ICE(Resolver, diagnostics_)
+ << "Validator::Variable() called with a unknown variable type: "
+ << decl->TypeInfo().name;
+ return false;
+ });
+}
+
bool Validator::GlobalVariable(
const sem::GlobalVariable* global,
std::unordered_map<uint32_t, const sem::Variable*> constant_ids,
@@ -583,6 +609,14 @@
return false;
}
+ auto name = symbols_.NameFor(var->symbol);
+ if (sem::ParseBuiltinType(name) != sem::BuiltinType::kNone) {
+ AddError(
+ "'" + name + "' is a builtin and cannot be redeclared as a module-scope 'var'",
+ var->source);
+ return false;
+ }
+
return Var(global);
},
[&](const ast::Override*) {
@@ -702,49 +736,15 @@
return true;
}
-bool Validator::Variable(const sem::Variable* v) const {
- auto* decl = v->Declaration();
- return Switch(
- decl, //
- [&](const ast::Var*) { return Var(v); }, //
- [&](const ast::Let*) { return Let(v); }, //
- [&](const ast::Override*) { return Override(v); }, //
- [&](const ast::Const*) { return true; }, //
- [&](Default) {
- TINT_ICE(Resolver, diagnostics_)
- << "Validator::Variable() called with a unknown variable type: "
- << decl->TypeInfo().name;
- return false;
- });
-}
-
bool Validator::Var(const sem::Variable* v) const {
auto* var = v->Declaration()->As<ast::Var>();
auto* storage_ty = v->Type()->UnwrapRef();
- if (v->Is<sem::GlobalVariable>()) {
- auto name = symbols_.NameFor(var->symbol);
- if (sem::ParseBuiltinType(name) != sem::BuiltinType::kNone) {
- AddError("'" + name + "' is a builtin and cannot be redeclared as a module-scope 'var'",
- var->source);
- return false;
- }
- }
-
if (!IsStorable(storage_ty)) {
AddError(sem_.TypeNameOf(storage_ty) + " cannot be used as the type of a var", var->source);
return false;
}
- if (v->Is<sem::LocalVariable>() &&
- IsValidationEnabled(var->attributes, ast::DisabledValidation::kIgnoreStorageClass)) {
- if (!v->Type()->UnwrapRef()->IsConstructible()) {
- AddError("function-scope 'var' must have a constructible type",
- var->type ? var->type->source : var->source);
- return false;
- }
- }
-
if (storage_ty->is_handle() && var->declared_storage_class != ast::StorageClass::kNone) {
// https://gpuweb.github.io/gpuweb/wgsl/#module-scope-variables
// If the store type is a texture type or a sampler type, then the
diff --git a/src/tint/resolver/validator.h b/src/tint/resolver/validator.h
index a591589..4d41959 100644
--- a/src/tint/resolver/validator.h
+++ b/src/tint/resolver/validator.h
@@ -264,6 +264,11 @@
/// @returns true on success, false otherwise.
bool BuiltinCall(const sem::Call* call) const;
+ /// Validates a local variable
+ /// @param v the variable to validate
+ /// @returns true on success, false otherwise.
+ bool LocalVariable(const sem::Variable* v) const;
+
/// Validates a location attribute
/// @param location the location attribute to validate
/// @param type the variable type
@@ -354,11 +359,6 @@
/// @returns true on success, false otherwise
bool SwitchStatement(const ast::SwitchStatement* s);
- /// Validates a variable
- /// @param v the variable to validate
- /// @returns true on success, false otherwise.
- bool Variable(const sem::Variable* v) const;
-
/// Validates a 'var' variable declaration
/// @param v the variable to validate
/// @returns true on success, false otherwise.
diff --git a/src/tint/resolver/variable_validation_test.cc b/src/tint/resolver/variable_validation_test.cc
index b538db0..5625f00 100644
--- a/src/tint/resolver/variable_validation_test.cc
+++ b/src/tint/resolver/variable_validation_test.cc
@@ -59,18 +59,16 @@
EXPECT_EQ(r()->error(), "12:34 error: override declaration requires a type or initializer");
}
-TEST_F(ResolverVariableValidationTest, VarTypeNotStorable) {
+TEST_F(ResolverVariableValidationTest, VarTypeNotConstructible) {
// var i : i32;
// var p : pointer<function, i32> = &v;
auto* i = Var("i", ty.i32(), ast::StorageClass::kNone);
- auto* p = Var(Source{{56, 78}}, "a", ty.pointer<i32>(ast::StorageClass::kFunction),
+ auto* p = Var("a", ty.pointer<i32>(Source{{56, 78}}, ast::StorageClass::kFunction),
ast::StorageClass::kNone, AddressOf(Source{{12, 34}}, "i"));
WrapInFunction(i, p);
EXPECT_FALSE(r()->Resolve());
- EXPECT_EQ(r()->error(),
- "56:78 error: ptr<function, i32, read_write> cannot be used as the "
- "type of a var");
+ EXPECT_EQ(r()->error(), "56:78 error: function-scope 'var' must have a constructible type");
}
TEST_F(ResolverVariableValidationTest, LetTypeNotConstructible) {
