From aedca4288c045b4face835e8210ebc1490e39a7e Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Mon, 30 Nov 2020 23:30:58 +0000 Subject: [PATCH] Replace Variable::(Is|As)* with Castable Change-Id: I7a49287af079d53cee095fa2243dd21757546b56 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/34318 Reviewed-by: dan sinclair --- src/ast/decorated_variable.cc | 4 --- src/ast/decorated_variable.h | 3 -- src/ast/decorated_variable_test.cc | 2 +- src/ast/function.cc | 35 ++++++++++--------- src/ast/variable.cc | 14 -------- src/ast/variable.h | 10 ------ src/inspector/inspector.cc | 4 +-- .../parser_impl_global_variable_decl_test.cc | 16 ++++----- src/transform/vertex_pulling_transform.cc | 15 ++++---- src/writer/hlsl/generator_impl.cc | 16 +++++---- src/writer/msl/generator_impl.cc | 18 ++++++---- src/writer/spirv/builder.cc | 26 +++++++------- src/writer/wgsl/generator_impl.cc | 4 +-- src/writer/wgsl/generator_impl.h | 1 + 14 files changed, 75 insertions(+), 93 deletions(-) diff --git a/src/ast/decorated_variable.cc b/src/ast/decorated_variable.cc index 5e5694a561..447269294a 100644 --- a/src/ast/decorated_variable.cc +++ b/src/ast/decorated_variable.cc @@ -69,10 +69,6 @@ uint32_t DecoratedVariable::constant_id() const { return 0; } -bool DecoratedVariable::IsDecorated() const { - return true; -} - bool DecoratedVariable::IsValid() const { return Variable::IsValid(); } diff --git a/src/ast/decorated_variable.h b/src/ast/decorated_variable.h index 1eeea8c09d..b0067b5e45 100644 --- a/src/ast/decorated_variable.h +++ b/src/ast/decorated_variable.h @@ -56,9 +56,6 @@ class DecoratedVariable : public Castable { /// |HasConstantIdDecoration| has been called first. uint32_t constant_id() const; - /// @returns true if this is a decorated variable - bool IsDecorated() const override; - /// @returns true if the name and path are both present bool IsValid() const override; diff --git a/src/ast/decorated_variable_test.cc b/src/ast/decorated_variable_test.cc index f133a132b9..2b41652945 100644 --- a/src/ast/decorated_variable_test.cc +++ b/src/ast/decorated_variable_test.cc @@ -108,7 +108,7 @@ TEST_F(DecoratedVariableTest, IsValid) { TEST_F(DecoratedVariableTest, IsDecorated) { DecoratedVariable dv; - EXPECT_TRUE(dv.IsDecorated()); + EXPECT_TRUE(dv.Is()); } TEST_F(DecoratedVariableTest, to_str) { diff --git a/src/ast/function.cc b/src/ast/function.cc index b636559cd7..8c72209d55 100644 --- a/src/ast/function.cc +++ b/src/ast/function.cc @@ -85,13 +85,12 @@ Function::referenced_location_variables() const { std::vector> ret; for (auto* var : referenced_module_variables()) { - if (!var->IsDecorated()) { - continue; - } - for (auto* deco : var->AsDecorated()->decorations()) { - if (auto* location = deco->As()) { - ret.push_back({var, location}); - break; + if (auto* decos = var->As()) { + for (auto* deco : decos->decorations()) { + if (auto* location = deco->As()) { + ret.push_back({var, location}); + break; + } } } } @@ -103,14 +102,14 @@ Function::referenced_uniform_variables() const { std::vector> ret; for (auto* var : referenced_module_variables()) { - if (!var->IsDecorated() || + if (!var->Is() || var->storage_class() != ast::StorageClass::kUniform) { continue; } BindingDecoration* binding = nullptr; SetDecoration* set = nullptr; - for (auto* deco : var->AsDecorated()->decorations()) { + for (auto* deco : var->As()->decorations()) { if (auto* b = deco->As()) { binding = b; } else if (auto* s = deco->As()) { @@ -131,14 +130,14 @@ Function::referenced_storagebuffer_variables() const { std::vector> ret; for (auto* var : referenced_module_variables()) { - if (!var->IsDecorated() || + if (!var->Is() || var->storage_class() != ast::StorageClass::kStorageBuffer) { continue; } BindingDecoration* binding = nullptr; SetDecoration* set = nullptr; - for (auto* deco : var->AsDecorated()->decorations()) { + for (auto* deco : var->As()->decorations()) { if (auto* b = deco->As()) { binding = b; } else if (auto* s = deco->As()) { @@ -159,10 +158,10 @@ Function::referenced_builtin_variables() const { std::vector> ret; for (auto* var : referenced_module_variables()) { - if (!var->IsDecorated()) { + if (!var->Is()) { continue; } - for (auto* deco : var->AsDecorated()->decorations()) { + for (auto* deco : var->As()->decorations()) { if (auto* builtin = deco->As()) { ret.push_back({var, builtin}); break; @@ -283,14 +282,15 @@ Function::ReferencedSamplerVariablesImpl(type::SamplerKind kind) const { for (auto* var : referenced_module_variables()) { auto* unwrapped_type = var->type()->UnwrapIfNeeded(); - if (!var->IsDecorated() || !unwrapped_type->Is() || + if (!var->Is() || + !unwrapped_type->Is() || unwrapped_type->As()->kind() != kind) { continue; } BindingDecoration* binding = nullptr; SetDecoration* set = nullptr; - for (auto* deco : var->AsDecorated()->decorations()) { + for (auto* deco : var->As()->decorations()) { if (auto* b = deco->As()) { binding = b; } else if (auto* s = deco->As()) { @@ -312,7 +312,8 @@ Function::ReferencedSampledTextureVariablesImpl(bool multisampled) const { for (auto* var : referenced_module_variables()) { auto* unwrapped_type = var->type()->UnwrapIfNeeded(); - if (!var->IsDecorated() || !unwrapped_type->Is()) { + if (!var->Is() || + !unwrapped_type->Is()) { continue; } @@ -325,7 +326,7 @@ Function::ReferencedSampledTextureVariablesImpl(bool multisampled) const { BindingDecoration* binding = nullptr; SetDecoration* set = nullptr; - for (auto* deco : var->AsDecorated()->decorations()) { + for (auto* deco : var->As()->decorations()) { if (auto* b = deco->As()) { binding = b; } else if (auto* s = deco->As()) { diff --git a/src/ast/variable.cc b/src/ast/variable.cc index 67b2ed167e..37c1f207cf 100644 --- a/src/ast/variable.cc +++ b/src/ast/variable.cc @@ -36,20 +36,6 @@ Variable::Variable(Variable&&) = default; Variable::~Variable() = default; -DecoratedVariable* Variable::AsDecorated() { - assert(IsDecorated()); - return static_cast(this); -} - -const DecoratedVariable* Variable::AsDecorated() const { - assert(IsDecorated()); - return static_cast(this); -} - -bool Variable::IsDecorated() const { - return false; -} - bool Variable::IsValid() const { if (name_.length() == 0) { return false; diff --git a/src/ast/variable.h b/src/ast/variable.h index 6445946711..11c95d3847 100644 --- a/src/ast/variable.h +++ b/src/ast/variable.h @@ -29,8 +29,6 @@ namespace tint { namespace ast { -class DecoratedVariable; - /// A Variable statement. /// /// An instance of this class represents one of three constructs in WGSL: "var" @@ -134,14 +132,6 @@ class Variable : public Castable { /// @returns true if this is a constant, false otherwise bool is_const() const { return is_const_; } - /// @returns true if this is a decorated variable - virtual bool IsDecorated() const; - - /// @returns the expression as a decorated variable - DecoratedVariable* AsDecorated(); - /// @returns the expression as a decorated variable - const DecoratedVariable* AsDecorated() const; - /// @returns true if the name and path are both present bool IsValid() const override; diff --git a/src/inspector/inspector.cc b/src/inspector/inspector.cc index a9401a4fa8..9012f73efa 100644 --- a/src/inspector/inspector.cc +++ b/src/inspector/inspector.cc @@ -100,11 +100,11 @@ std::string Inspector::GetRemappedNameForEntryPoint( std::map Inspector::GetConstantIDs() { std::map result; for (auto* var : module_.global_variables()) { - if (!var->IsDecorated()) { + auto* decorated = var->As(); + if (decorated == nullptr) { continue; } - auto* decorated = var->AsDecorated(); if (!decorated->HasConstantIdDecoration()) { continue; } diff --git a/src/reader/wgsl/parser_impl_global_variable_decl_test.cc b/src/reader/wgsl/parser_impl_global_variable_decl_test.cc index b03cd79697..76f2c29751 100644 --- a/src/reader/wgsl/parser_impl_global_variable_decl_test.cc +++ b/src/reader/wgsl/parser_impl_global_variable_decl_test.cc @@ -46,7 +46,7 @@ TEST_F(ParserImplTest, GlobalVariableDecl_WithoutConstructor) { EXPECT_EQ(e->source().range.end.column, 11u); ASSERT_EQ(e->constructor(), nullptr); - ASSERT_FALSE(e->IsDecorated()); + ASSERT_FALSE(e->Is()); } TEST_F(ParserImplTest, GlobalVariableDecl_WithConstructor) { @@ -73,7 +73,7 @@ TEST_F(ParserImplTest, GlobalVariableDecl_WithConstructor) { ASSERT_TRUE(e->constructor()->Is()); ASSERT_TRUE(e->constructor()->Is()); - ASSERT_FALSE(e->IsDecorated()); + ASSERT_FALSE(e->Is()); } TEST_F(ParserImplTest, GlobalVariableDecl_WithDecoration) { @@ -86,7 +86,7 @@ TEST_F(ParserImplTest, GlobalVariableDecl_WithDecoration) { EXPECT_TRUE(e.matched); EXPECT_FALSE(e.errored); ASSERT_NE(e.value, nullptr); - ASSERT_TRUE(e->IsDecorated()); + ASSERT_TRUE(e->Is()); EXPECT_EQ(e->name(), "a"); ASSERT_NE(e->type(), nullptr); @@ -100,8 +100,8 @@ TEST_F(ParserImplTest, GlobalVariableDecl_WithDecoration) { ASSERT_EQ(e->constructor(), nullptr); - ASSERT_TRUE(e->IsDecorated()); - auto* v = e->AsDecorated(); + ASSERT_TRUE(e->Is()); + auto* v = e->As(); auto& decorations = v->decorations(); ASSERT_EQ(decorations.size(), 2u); @@ -120,7 +120,7 @@ TEST_F(ParserImplTest, GlobalVariableDecl_WithDecoration_MulitpleGroups) { EXPECT_TRUE(e.matched); EXPECT_FALSE(e.errored); ASSERT_NE(e.value, nullptr); - ASSERT_TRUE(e->IsDecorated()); + ASSERT_TRUE(e->Is()); EXPECT_EQ(e->name(), "a"); ASSERT_NE(e->type(), nullptr); @@ -134,8 +134,8 @@ TEST_F(ParserImplTest, GlobalVariableDecl_WithDecoration_MulitpleGroups) { ASSERT_EQ(e->constructor(), nullptr); - ASSERT_TRUE(e->IsDecorated()); - auto* v = e->AsDecorated(); + ASSERT_TRUE(e->Is()); + auto* v = e->As(); auto& decorations = v->decorations(); ASSERT_EQ(decorations.size(), 2u); diff --git a/src/transform/vertex_pulling_transform.cc b/src/transform/vertex_pulling_transform.cc index 11544cad32..40cfc4abd2 100644 --- a/src/transform/vertex_pulling_transform.cc +++ b/src/transform/vertex_pulling_transform.cc @@ -122,11 +122,12 @@ void VertexPullingTransform::FindOrInsertVertexIndexIfUsed() { // Look for an existing vertex index builtin for (auto* v : mod_->global_variables()) { - if (!v->IsDecorated() || v->storage_class() != ast::StorageClass::kInput) { + if (!v->Is() || + v->storage_class() != ast::StorageClass::kInput) { continue; } - for (auto* d : v->AsDecorated()->decorations()) { + for (auto* d : v->As()->decorations()) { if (d->Is() && d->As()->value() == ast::Builtin::kVertexIdx) { @@ -165,11 +166,12 @@ void VertexPullingTransform::FindOrInsertInstanceIndexIfUsed() { // Look for an existing instance index builtin for (auto* v : mod_->global_variables()) { - if (!v->IsDecorated() || v->storage_class() != ast::StorageClass::kInput) { + if (!v->Is() || + v->storage_class() != ast::StorageClass::kInput) { continue; } - for (auto* d : v->AsDecorated()->decorations()) { + for (auto* d : v->As()->decorations()) { if (d->Is() && d->As()->value() == ast::Builtin::kInstanceIdx) { @@ -195,11 +197,12 @@ void VertexPullingTransform::FindOrInsertInstanceIndexIfUsed() { void VertexPullingTransform::ConvertVertexInputVariablesToPrivate() { for (auto*& v : mod_->global_variables()) { - if (!v->IsDecorated() || v->storage_class() != ast::StorageClass::kInput) { + if (!v->Is() || + v->storage_class() != ast::StorageClass::kInput) { continue; } - for (auto* d : v->AsDecorated()->decorations()) { + for (auto* d : v->As()->decorations()) { if (auto* l = d->As()) { uint32_t location = l->value(); // This is where the replacement happens. Expressions use identifier diff --git a/src/writer/hlsl/generator_impl.cc b/src/writer/hlsl/generator_impl.cc index c5c61b5186..a84af8cfeb 100644 --- a/src/writer/hlsl/generator_impl.cc +++ b/src/writer/hlsl/generator_impl.cc @@ -1005,9 +1005,9 @@ bool GeneratorImpl::EmitExpression(std::ostream& pre, } bool GeneratorImpl::global_is_in_struct(ast::Variable* var) const { - return var->IsDecorated() && - (var->AsDecorated()->HasLocationDecoration() || - var->AsDecorated()->HasBuiltinDecoration()) && + return var->Is() && + (var->As()->HasLocationDecoration() || + var->As()->HasBuiltinDecoration()) && (var->storage_class() == ast::StorageClass::kInput || var->storage_class() == ast::StorageClass::kOutput); } @@ -2219,7 +2219,7 @@ bool GeneratorImpl::EmitVariable(std::ostream& out, make_indent(out); // TODO(dsinclair): Handle variable decorations - if (var->IsDecorated()) { + if (var->Is()) { error_ = "Variable decorations are not handled yet"; return false; } @@ -2253,7 +2253,8 @@ bool GeneratorImpl::EmitProgramConstVariable(std::ostream& out, const ast::Variable* var) { make_indent(out); - if (var->IsDecorated() && !var->AsDecorated()->HasConstantIdDecoration()) { + if (var->Is() && + !var->As()->HasConstantIdDecoration()) { error_ = "Decorated const values not valid"; return false; } @@ -2271,8 +2272,9 @@ bool GeneratorImpl::EmitProgramConstVariable(std::ostream& out, out << pre.str(); } - if (var->IsDecorated() && var->AsDecorated()->HasConstantIdDecoration()) { - auto const_id = var->AsDecorated()->constant_id(); + if (var->Is() && + var->As()->HasConstantIdDecoration()) { + auto const_id = var->As()->constant_id(); out << "#ifndef WGSL_SPEC_CONSTANT_" << const_id << std::endl; diff --git a/src/writer/msl/generator_impl.cc b/src/writer/msl/generator_impl.cc index a1a9d0d714..d4826e99bb 100644 --- a/src/writer/msl/generator_impl.cc +++ b/src/writer/msl/generator_impl.cc @@ -1492,11 +1492,13 @@ bool GeneratorImpl::EmitEntryPointFunction(ast::Function* func) { bool GeneratorImpl::global_is_in_struct(ast::Variable* var) const { bool in_or_out_struct_has_location = - var->IsDecorated() && var->AsDecorated()->HasLocationDecoration() && + var->Is() && + var->As()->HasLocationDecoration() && (var->storage_class() == ast::StorageClass::kInput || var->storage_class() == ast::StorageClass::kOutput); bool in_struct_has_builtin = - var->IsDecorated() && var->AsDecorated()->HasBuiltinDecoration() && + var->Is() && + var->As()->HasBuiltinDecoration() && var->storage_class() == ast::StorageClass::kOutput; return in_or_out_struct_has_location || in_struct_has_builtin; } @@ -2006,7 +2008,7 @@ bool GeneratorImpl::EmitVariable(ast::Variable* var, bool skip_constructor) { make_indent(); // TODO(dsinclair): Handle variable decorations - if (var->IsDecorated()) { + if (var->Is()) { error_ = "Variable decorations are not handled yet"; return false; } @@ -2044,7 +2046,8 @@ bool GeneratorImpl::EmitVariable(ast::Variable* var, bool skip_constructor) { bool GeneratorImpl::EmitProgramConstVariable(const ast::Variable* var) { make_indent(); - if (var->IsDecorated() && !var->AsDecorated()->HasConstantIdDecoration()) { + if (var->Is() && + !var->As()->HasConstantIdDecoration()) { error_ = "Decorated const values not valid"; return false; } @@ -2061,9 +2064,10 @@ bool GeneratorImpl::EmitProgramConstVariable(const ast::Variable* var) { out_ << " " << var->name(); } - if (var->IsDecorated() && var->AsDecorated()->HasConstantIdDecoration()) { - out_ << " [[function_constant(" << var->AsDecorated()->constant_id() - << ")]]"; + if (var->Is() && + var->As()->HasConstantIdDecoration()) { + out_ << " [[function_constant(" + << var->As()->constant_id() << ")]]"; } else if (var->constructor() != nullptr) { out_ << " = "; if (!EmitExpression(var->constructor())) { diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc index 1f2bdc2af6..042bea578f 100644 --- a/src/writer/spirv/builder.cc +++ b/src/writer/spirv/builder.cc @@ -747,7 +747,8 @@ bool Builder::GenerateGlobalVariable(ast::Variable* var) { // one // 2- If we don't have a constructor and we're an Output or Private variable // then WGSL requires an initializer. - if (var->IsDecorated() && var->AsDecorated()->HasConstantIdDecoration()) { + if (var->Is() && + var->As()->HasConstantIdDecoration()) { if (type->Is()) { ast::FloatLiteral l(type, 0.0f); init_id = GenerateLiteralIfNeeded(var, &l); @@ -782,8 +783,8 @@ bool Builder::GenerateGlobalVariable(ast::Variable* var) { push_type(spv::Op::OpVariable, std::move(ops)); - if (var->IsDecorated()) { - for (auto* deco : var->AsDecorated()->decorations()) { + if (auto* decorated = var->As()) { + for (auto* deco : decorated->decorations()) { if (auto* builtin = deco->As()) { push_annot(spv::Op::OpDecorate, {Operand::Int(var_id), Operand::Int(SpvDecorationBuiltIn), @@ -1468,8 +1469,8 @@ uint32_t Builder::GenerateLiteralIfNeeded(ast::Variable* var, auto name = lit->name(); bool is_spec_constant = false; - if (var && var->IsDecorated() && - var->AsDecorated()->HasConstantIdDecoration()) { + if (var && var->Is() && + var->As()->HasConstantIdDecoration()) { name = "__spec" + name; is_spec_constant = true; } @@ -1483,9 +1484,10 @@ uint32_t Builder::GenerateLiteralIfNeeded(ast::Variable* var, auto result_id = result.to_i(); if (is_spec_constant) { - push_annot(spv::Op::OpDecorate, - {Operand::Int(result_id), Operand::Int(SpvDecorationSpecId), - Operand::Int(var->AsDecorated()->constant_id())}); + push_annot( + spv::Op::OpDecorate, + {Operand::Int(result_id), Operand::Int(SpvDecorationSpecId), + Operand::Int(var->As()->constant_id())}); } if (lit->IsBool()) { @@ -2701,10 +2703,10 @@ uint32_t Builder::GenerateStructMember(uint32_t struct_id, bool has_layout = false; for (auto* deco : member->decorations()) { if (auto* offset = deco->As()) { - push_annot(spv::Op::OpMemberDecorate, - {Operand::Int(struct_id), Operand::Int(idx), - Operand::Int(SpvDecorationOffset), - Operand::Int(offset->offset())}); + push_annot( + spv::Op::OpMemberDecorate, + {Operand::Int(struct_id), Operand::Int(idx), + Operand::Int(SpvDecorationOffset), Operand::Int(offset->offset())}); has_layout = true; } else { error_ = "unknown struct member decoration"; diff --git a/src/writer/wgsl/generator_impl.cc b/src/writer/wgsl/generator_impl.cc index 0761d805bc..88dfb21b70 100644 --- a/src/writer/wgsl/generator_impl.cc +++ b/src/writer/wgsl/generator_impl.cc @@ -602,8 +602,8 @@ bool GeneratorImpl::EmitStructType(const ast::type::StructType* str) { bool GeneratorImpl::EmitVariable(ast::Variable* var) { make_indent(); - if (var->IsDecorated()) { - if (!EmitVariableDecorations(var->AsDecorated())) { + if (auto* decorated = var->As()) { + if (!EmitVariableDecorations(decorated)) { return false; } } diff --git a/src/writer/wgsl/generator_impl.h b/src/writer/wgsl/generator_impl.h index 1de0b9d588..4680036254 100644 --- a/src/writer/wgsl/generator_impl.h +++ b/src/writer/wgsl/generator_impl.h @@ -27,6 +27,7 @@ #include "src/ast/case_statement.h" #include "src/ast/constructor_expression.h" #include "src/ast/continue_statement.h" +#include "src/ast/decorated_variable.h" #include "src/ast/discard_statement.h" #include "src/ast/fallthrough_statement.h" #include "src/ast/identifier_expression.h"