From c0eb9aeafbe777b3f617e82e5498af8e43e1affe Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Mon, 30 Nov 2020 23:30:58 +0000 Subject: [PATCH] Replace VariableDecoration::(Is|As)Builtin with Castable Change-Id: I49d970301c46cfe29d7b22e18abb443daa0c8073 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/34307 Reviewed-by: dan sinclair --- src/ast/binding_decoration_test.cc | 13 +++++++------ src/ast/builtin_decoration.cc | 4 ---- src/ast/builtin_decoration.h | 3 --- src/ast/builtin_decoration_test.cc | 2 +- src/ast/constant_id_decoration_test.cc | 2 +- src/ast/decorated_variable.cc | 3 ++- src/ast/function.cc | 4 ++-- src/ast/location_decoration_test.cc | 2 +- src/ast/set_decoration_test.cc | 2 +- src/ast/variable_decoration.cc | 9 --------- src/ast/variable_decoration.h | 5 ----- .../parser_impl_variable_decoration_list_test.cc | 5 +++-- .../wgsl/parser_impl_variable_decoration_test.cc | 4 ++-- src/transform/vertex_pulling_transform.cc | 10 ++++++---- src/writer/hlsl/generator_impl.cc | 10 ++++++---- src/writer/msl/generator_impl.cc | 5 +++-- src/writer/spirv/builder.cc | 4 ++-- src/writer/wgsl/generator_impl.cc | 4 ++-- 18 files changed, 39 insertions(+), 52 deletions(-) diff --git a/src/ast/binding_decoration_test.cc b/src/ast/binding_decoration_test.cc index 7204c37e73..7bd236f137 100644 --- a/src/ast/binding_decoration_test.cc +++ b/src/ast/binding_decoration_test.cc @@ -28,12 +28,13 @@ TEST_F(BindingDecorationTest, Creation) { } TEST_F(BindingDecorationTest, Is) { - BindingDecoration d{2, Source{}}; - EXPECT_TRUE(d.Is()); - EXPECT_FALSE(d.IsBuiltin()); - EXPECT_FALSE(d.IsConstantId()); - EXPECT_FALSE(d.IsLocation()); - EXPECT_FALSE(d.IsSet()); + BindingDecoration bd{2, Source{}}; + Decoration* d = &bd; + EXPECT_TRUE(d->Is()); + EXPECT_FALSE(d->Is()); + EXPECT_FALSE(bd.IsConstantId()); + EXPECT_FALSE(bd.IsLocation()); + EXPECT_FALSE(bd.IsSet()); } TEST_F(BindingDecorationTest, ToStr) { diff --git a/src/ast/builtin_decoration.cc b/src/ast/builtin_decoration.cc index 34d48f5210..856fc9a91b 100644 --- a/src/ast/builtin_decoration.cc +++ b/src/ast/builtin_decoration.cc @@ -22,10 +22,6 @@ BuiltinDecoration::BuiltinDecoration(Builtin builtin, const Source& source) BuiltinDecoration::~BuiltinDecoration() = default; -bool BuiltinDecoration::IsBuiltin() const { - return true; -} - void BuiltinDecoration::to_str(std::ostream& out, size_t indent) const { make_indent(out, indent); out << "BuiltinDecoration{" << builtin_ << "}" << std::endl; diff --git a/src/ast/builtin_decoration.h b/src/ast/builtin_decoration.h index 251a40946d..9f4636d937 100644 --- a/src/ast/builtin_decoration.h +++ b/src/ast/builtin_decoration.h @@ -31,9 +31,6 @@ class BuiltinDecoration BuiltinDecoration(Builtin builtin, const Source& source); ~BuiltinDecoration() override; - /// @returns true if this is a builtin decoration - bool IsBuiltin() const override; - /// @returns the builtin value Builtin value() const { return builtin_; } diff --git a/src/ast/builtin_decoration_test.cc b/src/ast/builtin_decoration_test.cc index 438552bc59..d3e3702424 100644 --- a/src/ast/builtin_decoration_test.cc +++ b/src/ast/builtin_decoration_test.cc @@ -31,7 +31,7 @@ TEST_F(BuiltinDecorationTest, Is) { BuiltinDecoration bd{Builtin::kFragDepth, Source{}}; Decoration* d = &bd; EXPECT_FALSE(d->Is()); - EXPECT_TRUE(bd.IsBuiltin()); + EXPECT_TRUE(d->Is()); EXPECT_FALSE(bd.IsConstantId()); EXPECT_FALSE(bd.IsLocation()); EXPECT_FALSE(bd.IsSet()); diff --git a/src/ast/constant_id_decoration_test.cc b/src/ast/constant_id_decoration_test.cc index f2b7c60de4..6df2495cce 100644 --- a/src/ast/constant_id_decoration_test.cc +++ b/src/ast/constant_id_decoration_test.cc @@ -31,7 +31,7 @@ TEST_F(ConstantIdDecorationTest, Is) { ConstantIdDecoration cd{27, Source{}}; Decoration* d = &cd; EXPECT_FALSE(d->Is()); - EXPECT_FALSE(cd.IsBuiltin()); + EXPECT_FALSE(d->Is()); EXPECT_TRUE(cd.IsConstantId()); EXPECT_FALSE(cd.IsLocation()); EXPECT_FALSE(cd.IsSet()); diff --git a/src/ast/decorated_variable.cc b/src/ast/decorated_variable.cc index 9b30e57a39..dcf6da375b 100644 --- a/src/ast/decorated_variable.cc +++ b/src/ast/decorated_variable.cc @@ -16,6 +16,7 @@ #include +#include "src/ast/builtin_decoration.h" #include "src/ast/constant_id_decoration.h" namespace tint { @@ -41,7 +42,7 @@ bool DecoratedVariable::HasLocationDecoration() const { bool DecoratedVariable::HasBuiltinDecoration() const { for (auto* deco : decorations_) { - if (deco->IsBuiltin()) { + if (deco->Is()) { return true; } } diff --git a/src/ast/function.cc b/src/ast/function.cc index f13a898070..555230bc89 100644 --- a/src/ast/function.cc +++ b/src/ast/function.cc @@ -163,8 +163,8 @@ Function::referenced_builtin_variables() const { continue; } for (auto* deco : var->AsDecorated()->decorations()) { - if (deco->IsBuiltin()) { - ret.push_back({var, deco->AsBuiltin()}); + if (auto* builtin = deco->As()) { + ret.push_back({var, builtin}); break; } } diff --git a/src/ast/location_decoration_test.cc b/src/ast/location_decoration_test.cc index d79492509d..e2c1b7cd35 100644 --- a/src/ast/location_decoration_test.cc +++ b/src/ast/location_decoration_test.cc @@ -33,7 +33,7 @@ TEST_F(LocationDecorationTest, Is) { LocationDecoration ld{2, Source{}}; Decoration* d = &ld; EXPECT_FALSE(d->Is()); - EXPECT_FALSE(ld.IsBuiltin()); + EXPECT_FALSE(d->Is()); EXPECT_FALSE(ld.IsConstantId()); EXPECT_TRUE(ld.IsLocation()); EXPECT_FALSE(ld.IsSet()); diff --git a/src/ast/set_decoration_test.cc b/src/ast/set_decoration_test.cc index e457573a35..df5afa1836 100644 --- a/src/ast/set_decoration_test.cc +++ b/src/ast/set_decoration_test.cc @@ -31,7 +31,7 @@ TEST_F(SetDecorationTest, Is) { SetDecoration sd{2, Source{}}; Decoration* d = &sd; EXPECT_FALSE(d->Is()); - EXPECT_FALSE(sd.IsBuiltin()); + EXPECT_FALSE(d->Is()); EXPECT_FALSE(sd.IsConstantId()); EXPECT_FALSE(sd.IsLocation()); EXPECT_TRUE(sd.IsSet()); diff --git a/src/ast/variable_decoration.cc b/src/ast/variable_decoration.cc index 5e902f164d..3dfb875bd2 100644 --- a/src/ast/variable_decoration.cc +++ b/src/ast/variable_decoration.cc @@ -35,10 +35,6 @@ DecorationKind VariableDecoration::GetKind() const { return Kind; } -bool VariableDecoration::IsBuiltin() const { - return false; -} - bool VariableDecoration::IsLocation() const { return false; } @@ -51,11 +47,6 @@ bool VariableDecoration::IsSet() const { return false; } -BuiltinDecoration* VariableDecoration::AsBuiltin() { - assert(IsBuiltin()); - return static_cast(this); -} - ConstantIdDecoration* VariableDecoration::AsConstantId() { assert(IsConstantId()); return static_cast(this); diff --git a/src/ast/variable_decoration.h b/src/ast/variable_decoration.h index a6b4f9b24b..c83d22be3a 100644 --- a/src/ast/variable_decoration.h +++ b/src/ast/variable_decoration.h @@ -25,7 +25,6 @@ namespace tint { namespace ast { -class BuiltinDecoration; class ConstantIdDecoration; class LocationDecoration; class SetDecoration; @@ -41,8 +40,6 @@ class VariableDecoration : public Castable { /// @return the decoration kind DecorationKind GetKind() const override; - /// @returns true if this is a builtin decoration - virtual bool IsBuiltin() const; /// @returns true if this is a constant id decoration virtual bool IsConstantId() const; /// @returns true if this is a location decoration @@ -50,8 +47,6 @@ class VariableDecoration : public Castable { /// @returns true if this is a set decoration virtual bool IsSet() const; - /// @returns the decoration as a builtin decoration - BuiltinDecoration* AsBuiltin(); /// @returns the decoration as a constant id decoration ConstantIdDecoration* AsConstantId(); /// @returns the decoration as a location decoration diff --git a/src/reader/wgsl/parser_impl_variable_decoration_list_test.cc b/src/reader/wgsl/parser_impl_variable_decoration_list_test.cc index 81f5356cb6..50b5e29d4c 100644 --- a/src/reader/wgsl/parser_impl_variable_decoration_list_test.cc +++ b/src/reader/wgsl/parser_impl_variable_decoration_list_test.cc @@ -38,8 +38,9 @@ TEST_F(ParserImplTest, VariableDecorationList_Parses) { ASSERT_TRUE(deco_0->IsLocation()); EXPECT_EQ(deco_0->AsLocation()->value(), 4u); - ASSERT_TRUE(deco_1->IsBuiltin()); - EXPECT_EQ(deco_1->AsBuiltin()->value(), ast::Builtin::kPosition); + ASSERT_TRUE(deco_1->Is()); + EXPECT_EQ(deco_1->As()->value(), + ast::Builtin::kPosition); } TEST_F(ParserImplTest, VariableDecorationList_Empty) { diff --git a/src/reader/wgsl/parser_impl_variable_decoration_test.cc b/src/reader/wgsl/parser_impl_variable_decoration_test.cc index 33ba645653..3c9cdcbe17 100644 --- a/src/reader/wgsl/parser_impl_variable_decoration_test.cc +++ b/src/reader/wgsl/parser_impl_variable_decoration_test.cc @@ -104,9 +104,9 @@ TEST_P(BuiltinTest, VariableDecoration_Builtin) { auto* var_deco = deco.value->As(); ASSERT_FALSE(p->has_error()) << p->error(); ASSERT_NE(var_deco, nullptr); - ASSERT_TRUE(var_deco->IsBuiltin()); + ASSERT_TRUE(var_deco->Is()); - auto* builtin = var_deco->AsBuiltin(); + auto* builtin = var_deco->As(); EXPECT_EQ(builtin->value(), params.result); } INSTANTIATE_TEST_SUITE_P( diff --git a/src/transform/vertex_pulling_transform.cc b/src/transform/vertex_pulling_transform.cc index 4d7a6e35be..05a6ab04ce 100644 --- a/src/transform/vertex_pulling_transform.cc +++ b/src/transform/vertex_pulling_transform.cc @@ -127,8 +127,9 @@ void VertexPullingTransform::FindOrInsertVertexIndexIfUsed() { } for (auto* d : v->AsDecorated()->decorations()) { - if (d->IsBuiltin() && - d->AsBuiltin()->value() == ast::Builtin::kVertexIdx) { + if (d->Is() && + d->As()->value() == + ast::Builtin::kVertexIdx) { vertex_index_name_ = v->name(); return; } @@ -169,8 +170,9 @@ void VertexPullingTransform::FindOrInsertInstanceIndexIfUsed() { } for (auto* d : v->AsDecorated()->decorations()) { - if (d->IsBuiltin() && - d->AsBuiltin()->value() == ast::Builtin::kInstanceIdx) { + if (d->Is() && + d->As()->value() == + ast::Builtin::kInstanceIdx) { instance_index_name_ = v->name(); return; } diff --git a/src/writer/hlsl/generator_impl.cc b/src/writer/hlsl/generator_impl.cc index b9f16d72ce..a8d14d418b 100644 --- a/src/writer/hlsl/generator_impl.cc +++ b/src/writer/hlsl/generator_impl.cc @@ -1383,8 +1383,9 @@ bool GeneratorImpl::EmitEntryPointData( return false; } out << "TEXCOORD" << deco->AsLocation()->value(); - } else if (deco->IsBuiltin()) { - auto attr = builtin_to_attribute(deco->AsBuiltin()->value()); + } else if (deco->Is()) { + auto attr = + builtin_to_attribute(deco->As()->value()); if (attr.empty()) { error_ = "unsupported builtin"; return false; @@ -1433,8 +1434,9 @@ bool GeneratorImpl::EmitEntryPointData( error_ = "invalid location variable for pipeline stage"; return false; } - } else if (deco->IsBuiltin()) { - auto attr = builtin_to_attribute(deco->AsBuiltin()->value()); + } else if (deco->Is()) { + auto attr = + builtin_to_attribute(deco->As()->value()); if (attr.empty()) { error_ = "unsupported builtin"; return false; diff --git a/src/writer/msl/generator_impl.cc b/src/writer/msl/generator_impl.cc index 0bd9116956..d38c9d98d7 100644 --- a/src/writer/msl/generator_impl.cc +++ b/src/writer/msl/generator_impl.cc @@ -1070,8 +1070,9 @@ bool GeneratorImpl::EmitEntryPointData(ast::Function* func) { error_ = "invalid location variable for pipeline stage"; return false; } - } else if (deco->IsBuiltin()) { - auto attr = builtin_to_attribute(deco->AsBuiltin()->value()); + } else if (deco->Is()) { + auto attr = + builtin_to_attribute(deco->As()->value()); if (attr.empty()) { error_ = "unsupported builtin"; return false; diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc index a4a4840ef2..c712310037 100644 --- a/src/writer/spirv/builder.cc +++ b/src/writer/spirv/builder.cc @@ -779,10 +779,10 @@ bool Builder::GenerateGlobalVariable(ast::Variable* var) { if (var->IsDecorated()) { for (auto* deco : var->AsDecorated()->decorations()) { - if (deco->IsBuiltin()) { + if (auto* builtin = deco->As()) { push_annot(spv::Op::OpDecorate, {Operand::Int(var_id), Operand::Int(SpvDecorationBuiltIn), - Operand::Int(ConvertBuiltin(deco->AsBuiltin()->value()))}); + Operand::Int(ConvertBuiltin(builtin->value()))}); } else if (deco->IsLocation()) { push_annot(spv::Op::OpDecorate, {Operand::Int(var_id), Operand::Int(SpvDecorationLocation), diff --git a/src/writer/wgsl/generator_impl.cc b/src/writer/wgsl/generator_impl.cc index e2dda1cf7c..b0d3fa5b81 100644 --- a/src/writer/wgsl/generator_impl.cc +++ b/src/writer/wgsl/generator_impl.cc @@ -647,8 +647,8 @@ bool GeneratorImpl::EmitVariableDecorations(ast::DecoratedVariable* var) { out_ << "set(" << deco->AsSet()->value() << ")"; } else if (deco->IsLocation()) { out_ << "location(" << deco->AsLocation()->value() << ")"; - } else if (deco->IsBuiltin()) { - out_ << "builtin(" << deco->AsBuiltin()->value() << ")"; + } else if (auto* builtin = deco->As()) { + out_ << "builtin(" << builtin->value() << ")"; } else if (deco->IsConstantId()) { out_ << "constant_id(" << deco->AsConstantId()->value() << ")"; } else {