tint/ast: Derive off `ast::Variable`
Add the new classes:
* `ast::Let`
* `ast::Override`
* `ast::Parameter`
* `ast::Var`
Limit the fields to those that are only applicable for their type.
Note: The resolver and validator is a tangled mess for each of the
variable types. This CL tries to keep the functionality exactly the
same. I'll clean this up in another change.
Bug: tint:1582
Change-Id: Iee83324167ffd4d92ae3032b2134677629c90079
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/93780
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
diff --git a/src/tint/resolver/dependency_graph.cc b/src/tint/resolver/dependency_graph.cc
index cf61149..6b3b779 100644
--- a/src/tint/resolver/dependency_graph.cc
+++ b/src/tint/resolver/dependency_graph.cc
@@ -487,11 +487,13 @@
/// declaration
std::string KindOf(const ast::Node* node) {
return Switch(
- node, //
- [&](const ast::Struct*) { return "struct"; },
- [&](const ast::Alias*) { return "alias"; },
- [&](const ast::Function*) { return "function"; },
- [&](const ast::Variable* var) { return var->is_const ? "let" : "var"; },
+ node, //
+ [&](const ast::Struct*) { return "struct"; }, //
+ [&](const ast::Alias*) { return "alias"; }, //
+ [&](const ast::Function*) { return "function"; }, //
+ [&](const ast::Let*) { return "let"; }, //
+ [&](const ast::Var*) { return "var"; }, //
+ [&](const ast::Override*) { return "override"; }, //
[&](Default) {
UnhandledNode(diagnostics_, node);
return "<error>";
diff --git a/src/tint/resolver/pipeline_overridable_constant_test.cc b/src/tint/resolver/pipeline_overridable_constant_test.cc
index 035936c3..583cc16 100644
--- a/src/tint/resolver/pipeline_overridable_constant_test.cc
+++ b/src/tint/resolver/pipeline_overridable_constant_test.cc
@@ -31,7 +31,7 @@
auto* sem = Sem().Get<sem::GlobalVariable>(var);
ASSERT_NE(sem, nullptr);
EXPECT_EQ(sem->Declaration(), var);
- EXPECT_TRUE(sem->IsOverridable());
+ EXPECT_TRUE(sem->Declaration()->Is<ast::Override>());
EXPECT_EQ(sem->ConstantId(), id);
EXPECT_FALSE(sem->ConstantValue());
}
@@ -45,7 +45,7 @@
auto* sem_a = Sem().Get<sem::GlobalVariable>(a);
ASSERT_NE(sem_a, nullptr);
EXPECT_EQ(sem_a->Declaration(), a);
- EXPECT_FALSE(sem_a->IsOverridable());
+ EXPECT_FALSE(sem_a->Declaration()->Is<ast::Override>());
EXPECT_TRUE(sem_a->ConstantValue());
}
diff --git a/src/tint/resolver/resolver.cc b/src/tint/resolver/resolver.cc
index 3d58930..3438a13 100644
--- a/src/tint/resolver/resolver.cc
+++ b/src/tint/resolver/resolver.cc
@@ -303,59 +303,63 @@
return s;
}
-sem::Variable* Resolver::Variable(const ast::Variable* var,
- VariableKind kind,
+sem::Variable* Resolver::Variable(const ast::Variable* v,
+ bool is_global,
uint32_t index /* = 0 */) {
const sem::Type* storage_ty = nullptr;
// If the variable has a declared type, resolve it.
- if (auto* ty = var->type) {
+ if (auto* ty = v->type) {
storage_ty = Type(ty);
if (!storage_ty) {
return nullptr;
}
}
+ auto* as_var = v->As<ast::Var>();
+ auto* as_let = v->As<ast::Let>();
+ auto* as_override = v->As<ast::Override>();
+ auto* as_param = v->As<ast::Parameter>();
+
const sem::Expression* rhs = nullptr;
// Does the variable have a constructor?
- if (var->constructor) {
- rhs = Materialize(Expression(var->constructor), storage_ty);
+ if (v->constructor) {
+ rhs = Materialize(Expression(v->constructor), storage_ty);
if (!rhs) {
return nullptr;
}
// If the variable has no declared type, infer it from the RHS
if (!storage_ty) {
- if (!var->is_const && kind == VariableKind::kGlobal) {
- AddError("module-scope 'var' declaration must specify a type", var->source);
+ if (as_var && is_global) {
+ AddError("module-scope 'var' declaration must specify a type", v->source);
return nullptr;
}
storage_ty = rhs->Type()->UnwrapRef(); // Implicit load of RHS
}
- } else if (var->is_const && !var->is_overridable && kind != VariableKind::kParameter) {
- AddError("'let' declaration must have an initializer", var->source);
+ } else if (as_let) {
+ AddError("'let' declaration must have an initializer", v->source);
return nullptr;
- } else if (!var->type) {
- AddError((kind == VariableKind::kGlobal)
- ? "module-scope 'var' declaration requires a type or initializer"
- : "function-scope 'var' declaration requires a type or initializer",
- var->source);
+ } else if (!v->type) {
+ AddError((is_global) ? "module-scope 'var' declaration requires a type or initializer"
+ : "function-scope 'var' declaration requires a type or initializer",
+ v->source);
return nullptr;
}
if (!storage_ty) {
TINT_ICE(Resolver, diagnostics_) << "failed to determine storage type for variable '" +
- builder_->Symbols().NameFor(var->symbol) + "'\n"
- << "Source: " << var->source;
+ builder_->Symbols().NameFor(v->symbol) + "'\n"
+ << "Source: " << v->source;
return nullptr;
}
- auto storage_class = var->declared_storage_class;
- if (storage_class == ast::StorageClass::kNone && !var->is_const) {
+ auto storage_class = as_var ? as_var->declared_storage_class : ast::StorageClass::kNone;
+ if (storage_class == ast::StorageClass::kNone && as_var) {
// No declared storage class. Infer from usage / type.
- if (kind == VariableKind::kLocal) {
+ if (!is_global) {
storage_class = ast::StorageClass::kFunction;
} else if (storage_ty->UnwrapRef()->is_handle()) {
// https://gpuweb.github.io/gpuweb/wgsl/#module-scope-variables
@@ -366,93 +370,83 @@
}
}
- if (kind == VariableKind::kLocal && !var->is_const &&
- storage_class != ast::StorageClass::kFunction &&
- validator_.IsValidationEnabled(var->attributes,
+ if (!is_global && as_var && storage_class != ast::StorageClass::kFunction &&
+ validator_.IsValidationEnabled(v->attributes,
ast::DisabledValidation::kIgnoreStorageClass)) {
- AddError("function-scope 'var' declaration must use 'function' storage class", var->source);
+ AddError("function-scope 'var' declaration must use 'function' storage class", v->source);
return nullptr;
}
- auto access = var->declared_access;
+ auto access = as_var ? as_var->declared_access : ast::Access::kUndefined;
if (access == ast::Access::kUndefined) {
access = DefaultAccessForStorageClass(storage_class);
}
auto* var_ty = storage_ty;
- if (!var->is_const) {
- // Variable declaration. Unlike `let`, `var` has storage.
+ if (as_var) {
+ // Variable declaration. Unlike `let` and parameters, `var` has storage.
// Variables are always of a reference type to the declared storage type.
var_ty = builder_->create<sem::Reference>(storage_ty, storage_class, access);
}
- if (rhs && !validator_.VariableConstructorOrCast(var, storage_class, storage_ty, rhs->Type())) {
+ if (rhs && !validator_.VariableConstructorOrCast(v, storage_class, storage_ty, rhs->Type())) {
return nullptr;
}
- if (!ApplyStorageClassUsageToType(storage_class, const_cast<sem::Type*>(var_ty), var->source)) {
- AddNote(std::string("while instantiating ") +
- ((kind == VariableKind::kParameter) ? "parameter " : "variable ") +
- builder_->Symbols().NameFor(var->symbol),
- var->source);
+ if (!ApplyStorageClassUsageToType(storage_class, const_cast<sem::Type*>(var_ty), v->source)) {
+ AddNote(std::string("while instantiating ") + ((as_param) ? "parameter " : "variable ") +
+ builder_->Symbols().NameFor(v->symbol),
+ v->source);
return nullptr;
}
- if (kind == VariableKind::kParameter) {
+ if (as_param) {
if (auto* ptr = var_ty->As<sem::Pointer>()) {
// For MSL, we push module-scope variables into the entry point as pointer
// parameters, so we also need to handle their store type.
if (!ApplyStorageClassUsageToType(
- ptr->StorageClass(), const_cast<sem::Type*>(ptr->StoreType()), var->source)) {
- AddNote("while instantiating parameter " + builder_->Symbols().NameFor(var->symbol),
- var->source);
+ ptr->StorageClass(), const_cast<sem::Type*>(ptr->StoreType()), v->source)) {
+ AddNote("while instantiating parameter " + builder_->Symbols().NameFor(v->symbol),
+ v->source);
return nullptr;
}
}
+ auto* param =
+ builder_->create<sem::Parameter>(as_param, index, var_ty, storage_class, access);
+ builder_->Sem().Add(as_param, param);
+ return param;
}
- switch (kind) {
- case VariableKind::kGlobal: {
- sem::BindingPoint binding_point;
- if (auto bp = var->BindingPoint()) {
+ if (is_global) {
+ sem::BindingPoint binding_point;
+ if (as_var) {
+ if (auto bp = as_var->BindingPoint()) {
binding_point = {bp.group->value, bp.binding->value};
}
+ }
- bool has_const_val = rhs && var->is_const && !var->is_overridable;
- auto* global = builder_->create<sem::GlobalVariable>(
- var, var_ty, storage_class, access,
- has_const_val ? rhs->ConstantValue() : sem::Constant{}, binding_point);
+ bool has_const_val = rhs && as_let && !as_override;
+ auto* global = builder_->create<sem::GlobalVariable>(
+ v, var_ty, storage_class, access,
+ has_const_val ? rhs->ConstantValue() : sem::Constant{}, binding_point);
- if (var->is_overridable) {
- global->SetIsOverridable();
- if (auto* id = ast::GetAttribute<ast::IdAttribute>(var->attributes)) {
- global->SetConstantId(static_cast<uint16_t>(id->value));
- }
+ if (as_override) {
+ if (auto* id = ast::GetAttribute<ast::IdAttribute>(v->attributes)) {
+ global->SetConstantId(static_cast<uint16_t>(id->value));
}
+ }
- global->SetConstructor(rhs);
-
- builder_->Sem().Add(var, global);
- return global;
- }
- case VariableKind::kLocal: {
- auto* local = builder_->create<sem::LocalVariable>(
- var, var_ty, storage_class, access, current_statement_,
- (rhs && var->is_const) ? rhs->ConstantValue() : sem::Constant{});
- builder_->Sem().Add(var, local);
- local->SetConstructor(rhs);
- return local;
- }
- case VariableKind::kParameter: {
- auto* param =
- builder_->create<sem::Parameter>(var, index, var_ty, storage_class, access);
- builder_->Sem().Add(var, param);
- return param;
- }
+ global->SetConstructor(rhs);
+ builder_->Sem().Add(v, global);
+ return global;
}
- TINT_UNREACHABLE(Resolver, diagnostics_) << "unhandled VariableKind " << static_cast<int>(kind);
- return nullptr;
+ auto* local = builder_->create<sem::LocalVariable>(
+ v, var_ty, storage_class, access, current_statement_,
+ (rhs && as_let) ? rhs->ConstantValue() : sem::Constant{});
+ builder_->Sem().Add(v, local);
+ local->SetConstructor(rhs);
+ return local;
}
ast::Access Resolver::DefaultAccessForStorageClass(ast::StorageClass storage_class) {
@@ -477,13 +471,13 @@
// TODO(crbug.com/tint/1192): If a transform changes the order or removes an
// unused constant, the allocation may change on the next Resolver pass.
for (auto* decl : builder_->AST().GlobalDeclarations()) {
- auto* var = decl->As<ast::Variable>();
- if (!var || !var->is_overridable) {
+ auto* override = decl->As<ast::Override>();
+ if (!override) {
continue;
}
uint16_t constant_id;
- if (auto* id_attr = ast::GetAttribute<ast::IdAttribute>(var->attributes)) {
+ if (auto* id_attr = ast::GetAttribute<ast::IdAttribute>(override->attributes)) {
constant_id = static_cast<uint16_t>(id_attr->value);
} else {
// No ID was specified, so allocate the next available ID.
@@ -499,7 +493,7 @@
next_constant_id = constant_id + 1;
}
- auto* sem = sem_.Get<sem::GlobalVariable>(var);
+ auto* sem = sem_.Get<sem::GlobalVariable>(override);
const_cast<sem::GlobalVariable*>(sem)->SetConstantId(constant_id);
}
}
@@ -513,25 +507,21 @@
}
}
-sem::GlobalVariable* Resolver::GlobalVariable(const ast::Variable* var) {
- auto* sem = Variable(var, VariableKind::kGlobal);
+sem::GlobalVariable* Resolver::GlobalVariable(const ast::Variable* v) {
+ auto* sem = As<sem::GlobalVariable>(Variable(v, /* is_global */ true));
if (!sem) {
return nullptr;
}
+ const bool is_var = v->Is<ast::Var>();
+
auto storage_class = sem->StorageClass();
- if (!var->is_const && storage_class == ast::StorageClass::kNone) {
- AddError("module-scope 'var' declaration must have a storage class", var->source);
- return nullptr;
- }
- if (var->is_const && storage_class != ast::StorageClass::kNone) {
- AddError(var->is_overridable ? "'override' declaration must not have a storage class"
- : "'let' declaration must not have a storage class",
- var->source);
+ if (is_var && storage_class == ast::StorageClass::kNone) {
+ AddError("module-scope 'var' declaration must have a storage class", v->source);
return nullptr;
}
- for (auto* attr : var->attributes) {
+ for (auto* attr : v->attributes) {
Mark(attr);
if (auto* id_attr = attr->As<ast::IdAttribute>()) {
@@ -540,7 +530,7 @@
}
}
- if (!validator_.NoDuplicateAttributes(var->attributes)) {
+ if (!validator_.NoDuplicateAttributes(v->attributes)) {
return nullptr;
}
@@ -576,9 +566,8 @@
}
}
- auto* var =
- As<sem::Parameter>(Variable(param, VariableKind::kParameter, parameter_index++));
- if (!var) {
+ auto* p = As<sem::Parameter>(Variable(param, false, parameter_index++));
+ if (!p) {
return nullptr;
}
@@ -589,10 +578,10 @@
return nullptr;
}
- parameters.emplace_back(var);
+ parameters.emplace_back(p);
- auto* var_ty = const_cast<sem::Type*>(var->Type());
- if (auto* str = var_ty->As<sem::Struct>()) {
+ auto* p_ty = const_cast<sem::Type*>(p->Type());
+ if (auto* str = p_ty->As<sem::Struct>()) {
switch (decl->PipelineStage()) {
case ast::PipelineStage::kVertex:
str->AddUsage(sem::PipelineStageUsage::kVertexInput);
@@ -777,12 +766,12 @@
if (auto* user = args[i]->As<sem::VariableUser>()) {
// We have an variable of a module-scope constant.
auto* decl = user->Variable()->Declaration();
- if (!decl->is_const) {
+ if (!decl->IsAnyOf<ast::Let, ast::Override>()) {
AddError(kErrBadType, values[i]->source);
return false;
}
// Capture the constant if it is pipeline-overridable.
- if (decl->is_overridable) {
+ if (decl->Is<ast::Override>()) {
ws[i].overridable_const = decl;
}
@@ -2104,19 +2093,19 @@
return nullptr;
}
+ constexpr const char* kErrInvalidExpr =
+ "array size identifier must be a literal or a module-scope 'let'";
+
if (auto* ident = count_expr->As<ast::IdentifierExpression>()) {
- // Make sure the identifier is a non-overridable module-scope constant.
- auto* var = sem_.ResolvedSymbol<sem::GlobalVariable>(ident);
- if (!var || !var->Declaration()->is_const || var->IsOverridable()) {
- AddError("array size identifier must be a literal or a module-scope 'let'",
- size_source);
+ // Make sure the identifier is a non-overridable module-scope 'let'.
+ auto* global = sem_.ResolvedSymbol<sem::GlobalVariable>(ident);
+ if (!global || !global->Declaration()->Is<ast::Let>()) {
+ AddError(kErrInvalidExpr, size_source);
return nullptr;
}
-
- count_expr = var->Declaration()->constructor;
+ count_expr = global->Declaration()->constructor;
} else if (!count_expr->Is<ast::LiteralExpression>()) {
- AddError("array size identifier must be a literal or a module-scope 'let'",
- size_source);
+ AddError(kErrInvalidExpr, size_source);
return nullptr;
}
@@ -2437,7 +2426,7 @@
return StatementScope(stmt, sem, [&] {
Mark(stmt->variable);
- auto* var = Variable(stmt->variable, VariableKind::kLocal);
+ auto* var = Variable(stmt->variable, /* is_global */ false);
if (!var) {
return false;
}
diff --git a/src/tint/resolver/resolver.h b/src/tint/resolver/resolver.h
index e619322..5f24169 100644
--- a/src/tint/resolver/resolver.h
+++ b/src/tint/resolver/resolver.h
@@ -109,9 +109,6 @@
const Validator* GetValidatorForTesting() const { return &validator_; }
private:
- /// Describes the context in which a variable is declared
- enum class VariableKind { kParameter, kLocal, kGlobal };
-
Validator::ValidTypeStorageLayouts valid_type_storage_layouts_;
/// Structure holding semantic information about a block (i.e. scope), such as
@@ -298,9 +295,9 @@
/// @note this method does not resolve the attributes as these are
/// context-dependent (global, local, parameter)
/// @param var the variable to create or return the `VariableInfo` for
- /// @param kind what kind of variable we are declaring
+ /// @param is_global true if this is module scope, otherwise function scope
/// @param index the index of the parameter, if this variable is a parameter
- sem::Variable* Variable(const ast::Variable* var, VariableKind kind, uint32_t index = 0);
+ sem::Variable* Variable(const ast::Variable* var, bool is_global, uint32_t index = 0);
/// Records the storage class usage for the given type, and any transient
/// dependencies of the type. Validates that the type can be used for the
diff --git a/src/tint/resolver/type_validation_test.cc b/src/tint/resolver/type_validation_test.cc
index 27ee04d..8f7ef1a 100644
--- a/src/tint/resolver/type_validation_test.cc
+++ b/src/tint/resolver/type_validation_test.cc
@@ -87,26 +87,6 @@
EXPECT_TRUE(r()->Resolve()) << r()->error();
}
-TEST_F(ResolverTypeValidationTest, GlobalLetWithStorageClass_Fail) {
- // let<private> global_var: f32;
- AST().AddGlobalVariable(create<ast::Variable>(
- Source{{12, 34}}, Symbols().Register("global_let"), ast::StorageClass::kPrivate,
- ast::Access::kUndefined, ty.f32(), true, false, Expr(1.23_f), ast::AttributeList{}));
-
- EXPECT_FALSE(r()->Resolve());
- EXPECT_EQ(r()->error(), "12:34 error: 'let' declaration must not have a storage class");
-}
-
-TEST_F(ResolverTypeValidationTest, OverrideWithStorageClass_Fail) {
- // let<private> global_var: f32;
- AST().AddGlobalVariable(create<ast::Variable>(
- Source{{12, 34}}, Symbols().Register("global_override"), ast::StorageClass::kPrivate,
- ast::Access::kUndefined, ty.f32(), true, true, Expr(1.23_f), ast::AttributeList{}));
-
- EXPECT_FALSE(r()->Resolve());
- EXPECT_EQ(r()->error(), "12:34 error: 'override' declaration must not have a storage class");
-}
-
TEST_F(ResolverTypeValidationTest, GlobalConstNoStorageClass_Pass) {
// let global_var: f32;
GlobalConst(Source{{12, 34}}, "global_var", ty.f32(), Construct(ty.f32()));
diff --git a/src/tint/resolver/uniformity.cc b/src/tint/resolver/uniformity.cc
index 18fa425..15ca72f 100644
--- a/src/tint/resolver/uniformity.cc
+++ b/src/tint/resolver/uniformity.cc
@@ -949,7 +949,7 @@
}
current_function_->variables.Set(sem_.Get(decl->variable), node);
- if (!decl->variable->is_const) {
+ if (decl->variable->Is<ast::Var>()) {
current_function_->local_var_decls.insert(
sem_.Get<sem::LocalVariable>(decl->variable));
}
@@ -1018,7 +1018,8 @@
},
[&](const sem::GlobalVariable* global) {
- if (global->Declaration()->is_const || global->Access() == ast::Access::kRead) {
+ if (!global->Declaration()->Is<ast::Var>() ||
+ global->Access() == ast::Access::kRead) {
node->AddEdge(cf);
} else {
node->AddEdge(current_function_->may_be_non_uniform);
diff --git a/src/tint/resolver/validator.cc b/src/tint/resolver/validator.cc
index 39eb31a..7562b79 100644
--- a/src/tint/resolver/validator.cc
+++ b/src/tint/resolver/validator.cc
@@ -297,7 +297,7 @@
return true;
}
-bool Validator::VariableConstructorOrCast(const ast::Variable* var,
+bool Validator::VariableConstructorOrCast(const ast::Variable* v,
ast::StorageClass storage_class,
const sem::Type* storage_ty,
const sem::Type* rhs_ty) const {
@@ -305,14 +305,14 @@
// Value type has to match storage type
if (storage_ty != value_type) {
- std::string decl = var->is_const ? "let" : "var";
+ std::string decl = v->Is<ast::Let>() ? "let" : "var";
AddError("cannot initialize " + decl + " of type '" + sem_.TypeNameOf(storage_ty) +
"' with value of type '" + sem_.TypeNameOf(rhs_ty) + "'",
- var->source);
+ v->source);
return false;
}
- if (!var->is_const) {
+ if (v->Is<ast::Var>()) {
switch (storage_class) {
case ast::StorageClass::kPrivate:
case ast::StorageClass::kFunction:
@@ -325,7 +325,7 @@
"' cannot have an initializer. var initializers are only "
"supported for the storage classes "
"'private' and 'function'",
- var->source);
+ v->source);
return false;
}
}
@@ -502,21 +502,22 @@
}
bool Validator::GlobalVariable(
- const sem::Variable* var,
+ const sem::GlobalVariable* global,
std::unordered_map<uint32_t, const sem::Variable*> constant_ids,
std::unordered_map<const sem::Type*, const Source&> atomic_composite_info) const {
- auto* decl = var->Declaration();
+ auto* decl = global->Declaration();
if (!NoDuplicateAttributes(decl->attributes)) {
return false;
}
- for (auto* attr : decl->attributes) {
- if (decl->is_const) {
- if (decl->is_overridable) {
+ bool ok = Switch(
+ decl, //
+ [&](const ast::Override*) {
+ for (auto* attr : decl->attributes) {
if (auto* id_attr = attr->As<ast::IdAttribute>()) {
uint32_t id = id_attr->value;
auto it = constant_ids.find(id);
- if (it != constant_ids.end() && it->second != var) {
+ if (it != constant_ids.end() && it->second != global) {
AddError("pipeline constant IDs must be unique", attr->source);
AddNote("a pipeline constant with an ID of " + std::to_string(id) +
" was previously declared here:",
@@ -533,32 +534,45 @@
AddError("attribute is not valid for 'override' declaration", attr->source);
return false;
}
- } else {
- AddError("attribute is not valid for module-scope 'let' declaration", attr->source);
+ }
+ return true;
+ },
+ [&](const ast::Let*) {
+ if (!decl->attributes.empty()) {
+ AddError("attribute is not valid for module-scope 'let' declaration",
+ decl->attributes[0]->source);
return false;
}
- } else {
- bool is_shader_io_attribute =
- attr->IsAnyOf<ast::BuiltinAttribute, ast::InterpolateAttribute,
- ast::InvariantAttribute, ast::LocationAttribute>();
- bool has_io_storage_class = var->StorageClass() == ast::StorageClass::kInput ||
- var->StorageClass() == ast::StorageClass::kOutput;
- if (!(attr->IsAnyOf<ast::BindingAttribute, ast::GroupAttribute,
- ast::InternalAttribute>()) &&
- (!is_shader_io_attribute || !has_io_storage_class)) {
- AddError("attribute is not valid for module-scope 'var'", attr->source);
- return false;
+ return true;
+ },
+ [&](const ast::Var*) {
+ for (auto* attr : decl->attributes) {
+ bool is_shader_io_attribute =
+ attr->IsAnyOf<ast::BuiltinAttribute, ast::InterpolateAttribute,
+ ast::InvariantAttribute, ast::LocationAttribute>();
+ bool has_io_storage_class = global->StorageClass() == ast::StorageClass::kInput ||
+ global->StorageClass() == ast::StorageClass::kOutput;
+ if (!attr->IsAnyOf<ast::BindingAttribute, ast::GroupAttribute,
+ ast::InternalAttribute>() &&
+ (!is_shader_io_attribute || !has_io_storage_class)) {
+ AddError("attribute is not valid for module-scope 'var'", attr->source);
+ return false;
+ }
}
- }
+ return true;
+ });
+
+ if (!ok) {
+ return false;
}
- if (var->StorageClass() == ast::StorageClass::kFunction) {
+ if (global->StorageClass() == ast::StorageClass::kFunction) {
AddError("module-scope 'var' must not use storage class 'function'", decl->source);
return false;
}
auto binding_point = decl->BindingPoint();
- switch (var->StorageClass()) {
+ switch (global->StorageClass()) {
case ast::StorageClass::kUniform:
case ast::StorageClass::kStorage:
case ast::StorageClass::kHandle: {
@@ -581,23 +595,23 @@
}
}
- // https://gpuweb.github.io/gpuweb/wgsl/#variable-declaration
- // The access mode always has a default, and except for variables in the
- // storage storage class, must not be written.
- if (var->StorageClass() != ast::StorageClass::kStorage &&
- decl->declared_access != ast::Access::kUndefined) {
- AddError("only variables in <storage> storage class may declare an access mode",
- decl->source);
- return false;
- }
+ if (auto* var = decl->As<ast::Var>()) {
+ // https://gpuweb.github.io/gpuweb/wgsl/#variable-declaration
+ // The access mode always has a default, and except for variables in the
+ // storage storage class, must not be written.
+ if (global->StorageClass() != ast::StorageClass::kStorage &&
+ var->declared_access != ast::Access::kUndefined) {
+ AddError("only variables in <storage> storage class may declare an access mode",
+ var->source);
+ return false;
+ }
- if (!decl->is_const) {
- if (!AtomicVariable(var, atomic_composite_info)) {
+ if (!AtomicVariable(global, atomic_composite_info)) {
return false;
}
}
- return Variable(var);
+ return Variable(global);
}
// https://gpuweb.github.io/gpuweb/wgsl/#atomic-types
@@ -641,14 +655,17 @@
return true;
}
-bool Validator::Variable(const sem::Variable* var) const {
- auto* decl = var->Declaration();
- auto* storage_ty = var->Type()->UnwrapRef();
+bool Validator::Variable(const sem::Variable* v) const {
+ auto* decl = v->Declaration();
+ auto* storage_ty = v->Type()->UnwrapRef();
- if (var->Is<sem::GlobalVariable>()) {
+ auto* as_let = decl->As<ast::Let>();
+ auto* as_var = decl->As<ast::Var>();
+
+ if (v->Is<sem::GlobalVariable>()) {
auto name = symbols_.NameFor(decl->symbol);
if (sem::ParseBuiltinType(name) != sem::BuiltinType::kNone) {
- auto* kind = var->Declaration()->is_const ? "let" : "var";
+ auto* kind = as_let ? "let" : "var";
AddError(
"'" + name + "' is a builtin and cannot be redeclared as a module-scope " + kind,
decl->source);
@@ -656,14 +673,13 @@
}
}
- if (!decl->is_const && !IsStorable(storage_ty)) {
+ if (as_var && !IsStorable(storage_ty)) {
AddError(sem_.TypeNameOf(storage_ty) + " cannot be used as the type of a var",
decl->source);
return false;
}
- if (decl->is_const && !var->Is<sem::Parameter>() &&
- !(storage_ty->IsConstructible() || storage_ty->Is<sem::Pointer>())) {
+ if (as_let && !(storage_ty->IsConstructible() || storage_ty->Is<sem::Pointer>())) {
AddError(sem_.TypeNameOf(storage_ty) + " cannot be used as the type of a let",
decl->source);
return false;
@@ -688,16 +704,17 @@
}
}
- if (var->Is<sem::LocalVariable>() && !decl->is_const &&
+ if (v->Is<sem::LocalVariable>() && as_var &&
IsValidationEnabled(decl->attributes, ast::DisabledValidation::kIgnoreStorageClass)) {
- if (!var->Type()->UnwrapRef()->IsConstructible()) {
+ if (!v->Type()->UnwrapRef()->IsConstructible()) {
AddError("function variable must have a constructible type",
decl->type ? decl->type->source : decl->source);
return false;
}
}
- if (storage_ty->is_handle() && decl->declared_storage_class != ast::StorageClass::kNone) {
+ if (as_var && storage_ty->is_handle() &&
+ as_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
// variable declaration must not have a storage class attribute. The
@@ -709,9 +726,10 @@
}
if (IsValidationEnabled(decl->attributes, ast::DisabledValidation::kIgnoreStorageClass) &&
- (decl->declared_storage_class == ast::StorageClass::kInput ||
- decl->declared_storage_class == ast::StorageClass::kOutput)) {
- AddError("invalid use of input/output storage class", decl->source);
+ as_var &&
+ (as_var->declared_storage_class == ast::StorageClass::kInput ||
+ as_var->declared_storage_class == ast::StorageClass::kOutput)) {
+ AddError("invalid use of input/output storage class", as_var->source);
return false;
}
return true;
@@ -1223,12 +1241,12 @@
// Validate there are no resource variable binding collisions
std::unordered_map<sem::BindingPoint, const ast::Variable*> binding_points;
- for (auto* var : func->TransitivelyReferencedGlobals()) {
- auto* var_decl = var->Declaration();
- if (!var_decl->BindingPoint()) {
+ for (auto* global : func->TransitivelyReferencedGlobals()) {
+ auto* var_decl = global->Declaration()->As<ast::Var>();
+ if (!var_decl || !var_decl->BindingPoint()) {
continue;
}
- auto bp = var->BindingPoint();
+ auto bp = global->BindingPoint();
auto res = binding_points.emplace(bp, var_decl);
if (!res.second &&
IsValidationEnabled(decl->attributes,
@@ -1663,12 +1681,6 @@
TINT_ICE(Resolver, diagnostics_) << "failed to resolve identifier";
return false;
}
- if (var->Declaration()->is_const) {
- TINT_ICE(Resolver, diagnostics_)
- << "Resolver::FunctionCall() encountered an address-of "
- "expression of a constant identifier expression";
- return false;
- }
is_valid = true;
}
}
@@ -2172,18 +2184,16 @@
// https://gpuweb.github.io/gpuweb/wgsl/#assignment-statement
auto const* lhs_ty = sem_.TypeOf(lhs);
- if (auto* var = sem_.ResolvedSymbol<sem::Variable>(lhs)) {
- auto* decl = var->Declaration();
- if (var->Is<sem::Parameter>()) {
- AddError("cannot assign to function parameter", lhs->source);
- AddNote("'" + symbols_.NameFor(decl->symbol) + "' is declared here:", decl->source);
- return false;
- }
- if (decl->is_const) {
- AddError(
- decl->is_overridable ? "cannot assign to 'override'" : "cannot assign to 'let'",
- lhs->source);
- AddNote("'" + symbols_.NameFor(decl->symbol) + "' is declared here:", decl->source);
+ if (auto* variable = sem_.ResolvedSymbol<sem::Variable>(lhs)) {
+ auto* v = variable->Declaration();
+ const char* err = Switch(
+ v, //
+ [&](const ast::Parameter*) { return "cannot assign to function parameter"; },
+ [&](const ast::Let*) { return "cannot assign to 'let'"; },
+ [&](const ast::Override*) { return "cannot assign to 'override'"; });
+ if (err) {
+ AddError(err, lhs->source);
+ AddNote("'" + symbols_.NameFor(v->symbol) + "' is declared here:", v->source);
return false;
}
}
@@ -2222,17 +2232,16 @@
// https://gpuweb.github.io/gpuweb/wgsl/#increment-decrement
- if (auto* var = sem_.ResolvedSymbol<sem::Variable>(lhs)) {
- auto* decl = var->Declaration();
- if (var->Is<sem::Parameter>()) {
- AddError("cannot modify function parameter", lhs->source);
- AddNote("'" + symbols_.NameFor(decl->symbol) + "' is declared here:", decl->source);
- return false;
- }
- if (decl->is_const) {
- AddError(decl->is_overridable ? "cannot modify 'override'" : "cannot modify 'let'",
- lhs->source);
- AddNote("'" + symbols_.NameFor(decl->symbol) + "' is declared here:", decl->source);
+ if (auto* variable = sem_.ResolvedSymbol<sem::Variable>(lhs)) {
+ auto* v = variable->Declaration();
+ const char* err = Switch(
+ v, //
+ [&](const ast::Parameter*) { return "cannot modify function parameter"; },
+ [&](const ast::Let*) { return "cannot modify 'let'"; },
+ [&](const ast::Override*) { return "cannot modify 'override'"; });
+ if (err) {
+ AddError(err, lhs->source);
+ AddNote("'" + symbols_.NameFor(v->symbol) + "' is declared here:", v->source);
return false;
}
}
diff --git a/src/tint/resolver/validator.h b/src/tint/resolver/validator.h
index e3912ba..223f3cf 100644
--- a/src/tint/resolver/validator.h
+++ b/src/tint/resolver/validator.h
@@ -237,7 +237,7 @@
/// @param atomic_composite_info atomic composite info in the module
/// @returns true on success, false otherwise
bool GlobalVariable(
- const sem::Variable* var,
+ const sem::GlobalVariable* var,
std::unordered_map<uint32_t, const sem::Variable*> constant_ids,
std::unordered_map<const sem::Type*, const Source&> atomic_composite_info) const;
@@ -345,12 +345,12 @@
bool Variable(const sem::Variable* var) const;
/// Validates a variable constructor or cast
- /// @param var the variable to validate
+ /// @param v the variable to validate
/// @param storage_class the storage class of the variable
/// @param storage_type the type of the storage
/// @param rhs_type the right hand side of the expression
/// @returns true on succes, false otherwise
- bool VariableConstructorOrCast(const ast::Variable* var,
+ bool VariableConstructorOrCast(const ast::Variable* v,
ast::StorageClass storage_class,
const sem::Type* storage_type,
const sem::Type* rhs_type) const;
diff --git a/src/tint/resolver/var_let_validation_test.cc b/src/tint/resolver/var_let_validation_test.cc
index eea5e72..67ecd53 100644
--- a/src/tint/resolver/var_let_validation_test.cc
+++ b/src/tint/resolver/var_let_validation_test.cc
@@ -24,22 +24,6 @@
struct ResolverVarLetValidationTest : public resolver::TestHelper, public testing::Test {};
-TEST_F(ResolverVarLetValidationTest, LetNoInitializer) {
- // let a : i32;
- WrapInFunction(Let(Source{{12, 34}}, "a", ty.i32(), nullptr));
-
- EXPECT_FALSE(r()->Resolve());
- EXPECT_EQ(r()->error(), "12:34 error: 'let' declaration must have an initializer");
-}
-
-TEST_F(ResolverVarLetValidationTest, GlobalLetNoInitializer) {
- // let a : i32;
- GlobalConst(Source{{12, 34}}, "a", ty.i32(), nullptr);
-
- EXPECT_FALSE(r()->Resolve());
- EXPECT_EQ(r()->error(), "12:34 error: 'let' declaration must have an initializer");
-}
-
TEST_F(ResolverVarLetValidationTest, VarNoInitializerNoType) {
// var a;
WrapInFunction(Var(Source{{12, 34}}, "a", nullptr));
