From 62d81485ddde7d0f178fbd8597f6ceb8e608bdb3 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)Binding with Castable Change-Id: Ia13ce42054e5f514eb34f6549da10b22129f9026 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/34306 Reviewed-by: dan sinclair --- src/ast/binding_decoration.cc | 4 --- src/ast/binding_decoration.h | 3 -- src/ast/binding_decoration_test.cc | 2 +- src/ast/builtin_decoration_test.cc | 13 ++++---- src/ast/constant_id_decoration_test.cc | 13 ++++---- src/ast/function.cc | 32 +++++++++---------- src/ast/location_decoration_test.cc | 13 ++++---- src/ast/set_decoration_test.cc | 13 ++++---- src/ast/variable_decoration.cc | 9 ------ src/ast/variable_decoration.h | 5 --- .../parser_impl_global_variable_decl_test.cc | 4 +-- .../parser_impl_variable_decoration_test.cc | 4 +-- src/writer/spirv/builder.cc | 4 +-- src/writer/wgsl/generator_impl.cc | 4 +-- 14 files changed, 53 insertions(+), 70 deletions(-) diff --git a/src/ast/binding_decoration.cc b/src/ast/binding_decoration.cc index 2b85ddac02..34e6023504 100644 --- a/src/ast/binding_decoration.cc +++ b/src/ast/binding_decoration.cc @@ -22,10 +22,6 @@ BindingDecoration::BindingDecoration(uint32_t val, const Source& source) BindingDecoration::~BindingDecoration() = default; -bool BindingDecoration::IsBinding() const { - return true; -} - void BindingDecoration::to_str(std::ostream& out, size_t indent) const { make_indent(out, indent); out << "BindingDecoration{" << value_ << "}" << std::endl; diff --git a/src/ast/binding_decoration.h b/src/ast/binding_decoration.h index c4bbf9dd32..ec2e756de3 100644 --- a/src/ast/binding_decoration.h +++ b/src/ast/binding_decoration.h @@ -32,9 +32,6 @@ class BindingDecoration BindingDecoration(uint32_t value, const Source& source); ~BindingDecoration() override; - /// @returns true if this is a binding decoration - bool IsBinding() const override; - /// @returns the binding value uint32_t value() const { return value_; } diff --git a/src/ast/binding_decoration_test.cc b/src/ast/binding_decoration_test.cc index 253d296633..7204c37e73 100644 --- a/src/ast/binding_decoration_test.cc +++ b/src/ast/binding_decoration_test.cc @@ -29,7 +29,7 @@ TEST_F(BindingDecorationTest, Creation) { TEST_F(BindingDecorationTest, Is) { BindingDecoration d{2, Source{}}; - EXPECT_TRUE(d.IsBinding()); + EXPECT_TRUE(d.Is()); EXPECT_FALSE(d.IsBuiltin()); EXPECT_FALSE(d.IsConstantId()); EXPECT_FALSE(d.IsLocation()); diff --git a/src/ast/builtin_decoration_test.cc b/src/ast/builtin_decoration_test.cc index b64fcd48d9..438552bc59 100644 --- a/src/ast/builtin_decoration_test.cc +++ b/src/ast/builtin_decoration_test.cc @@ -28,12 +28,13 @@ TEST_F(BuiltinDecorationTest, Creation) { } TEST_F(BuiltinDecorationTest, Is) { - BuiltinDecoration d{Builtin::kFragDepth, Source{}}; - EXPECT_FALSE(d.IsBinding()); - EXPECT_TRUE(d.IsBuiltin()); - EXPECT_FALSE(d.IsConstantId()); - EXPECT_FALSE(d.IsLocation()); - EXPECT_FALSE(d.IsSet()); + BuiltinDecoration bd{Builtin::kFragDepth, Source{}}; + Decoration* d = &bd; + EXPECT_FALSE(d->Is()); + EXPECT_TRUE(bd.IsBuiltin()); + EXPECT_FALSE(bd.IsConstantId()); + EXPECT_FALSE(bd.IsLocation()); + EXPECT_FALSE(bd.IsSet()); } TEST_F(BuiltinDecorationTest, ToStr) { diff --git a/src/ast/constant_id_decoration_test.cc b/src/ast/constant_id_decoration_test.cc index 32545fbb6d..f2b7c60de4 100644 --- a/src/ast/constant_id_decoration_test.cc +++ b/src/ast/constant_id_decoration_test.cc @@ -28,12 +28,13 @@ TEST_F(ConstantIdDecorationTest, Creation) { } TEST_F(ConstantIdDecorationTest, Is) { - ConstantIdDecoration d{27, Source{}}; - EXPECT_FALSE(d.IsBinding()); - EXPECT_FALSE(d.IsBuiltin()); - EXPECT_TRUE(d.IsConstantId()); - EXPECT_FALSE(d.IsLocation()); - EXPECT_FALSE(d.IsSet()); + ConstantIdDecoration cd{27, Source{}}; + Decoration* d = &cd; + EXPECT_FALSE(d->Is()); + EXPECT_FALSE(cd.IsBuiltin()); + EXPECT_TRUE(cd.IsConstantId()); + EXPECT_FALSE(cd.IsLocation()); + EXPECT_FALSE(cd.IsSet()); } TEST_F(ConstantIdDecorationTest, ToStr) { diff --git a/src/ast/function.cc b/src/ast/function.cc index da38be0e56..f13a898070 100644 --- a/src/ast/function.cc +++ b/src/ast/function.cc @@ -111,10 +111,10 @@ Function::referenced_uniform_variables() const { BindingDecoration* binding = nullptr; SetDecoration* set = nullptr; for (auto* deco : var->AsDecorated()->decorations()) { - if (deco->IsBinding()) { - binding = deco->AsBinding(); - } else if (deco->IsSet()) { - set = deco->AsSet(); + if (auto* b = deco->As()) { + binding = b; + } else if (auto* s = deco->As()) { + set = s; } } if (binding == nullptr || set == nullptr) { @@ -139,10 +139,10 @@ Function::referenced_storagebuffer_variables() const { BindingDecoration* binding = nullptr; SetDecoration* set = nullptr; for (auto* deco : var->AsDecorated()->decorations()) { - if (deco->IsBinding()) { - binding = deco->AsBinding(); - } else if (deco->IsSet()) { - set = deco->AsSet(); + if (auto* b = deco->As()) { + binding = b; + } else if (auto* s = deco->As()) { + set = s; } } if (binding == nullptr || set == nullptr) { @@ -291,10 +291,10 @@ Function::ReferencedSamplerVariablesImpl(type::SamplerKind kind) const { BindingDecoration* binding = nullptr; SetDecoration* set = nullptr; for (auto* deco : var->AsDecorated()->decorations()) { - if (deco->IsBinding()) { - binding = deco->AsBinding(); - } else if (deco->IsSet()) { - set = deco->AsSet(); + if (auto* b = deco->As()) { + binding = b; + } else if (auto* s = deco->As()) { + set = s; } } if (binding == nullptr || set == nullptr) { @@ -326,10 +326,10 @@ Function::ReferencedSampledTextureVariablesImpl(bool multisampled) const { BindingDecoration* binding = nullptr; SetDecoration* set = nullptr; for (auto* deco : var->AsDecorated()->decorations()) { - if (deco->IsBinding()) { - binding = deco->AsBinding(); - } else if (deco->IsSet()) { - set = deco->AsSet(); + if (auto* b = deco->As()) { + binding = b; + } else if (auto* s = deco->As()) { + set = s; } } if (binding == nullptr || set == nullptr) { diff --git a/src/ast/location_decoration_test.cc b/src/ast/location_decoration_test.cc index eec01d2475..d79492509d 100644 --- a/src/ast/location_decoration_test.cc +++ b/src/ast/location_decoration_test.cc @@ -30,12 +30,13 @@ TEST_F(LocationDecorationTest, Creation) { } TEST_F(LocationDecorationTest, Is) { - LocationDecoration d{2, Source{}}; - EXPECT_FALSE(d.IsBinding()); - EXPECT_FALSE(d.IsBuiltin()); - EXPECT_FALSE(d.IsConstantId()); - EXPECT_TRUE(d.IsLocation()); - EXPECT_FALSE(d.IsSet()); + LocationDecoration ld{2, Source{}}; + Decoration* d = &ld; + EXPECT_FALSE(d->Is()); + EXPECT_FALSE(ld.IsBuiltin()); + EXPECT_FALSE(ld.IsConstantId()); + EXPECT_TRUE(ld.IsLocation()); + EXPECT_FALSE(ld.IsSet()); } TEST_F(LocationDecorationTest, ToStr) { diff --git a/src/ast/set_decoration_test.cc b/src/ast/set_decoration_test.cc index acd43517e9..e457573a35 100644 --- a/src/ast/set_decoration_test.cc +++ b/src/ast/set_decoration_test.cc @@ -28,12 +28,13 @@ TEST_F(SetDecorationTest, Creation) { } TEST_F(SetDecorationTest, Is) { - SetDecoration d{2, Source{}}; - EXPECT_FALSE(d.IsBinding()); - EXPECT_FALSE(d.IsBuiltin()); - EXPECT_FALSE(d.IsConstantId()); - EXPECT_FALSE(d.IsLocation()); - EXPECT_TRUE(d.IsSet()); + SetDecoration sd{2, Source{}}; + Decoration* d = &sd; + EXPECT_FALSE(d->Is()); + EXPECT_FALSE(sd.IsBuiltin()); + EXPECT_FALSE(sd.IsConstantId()); + EXPECT_FALSE(sd.IsLocation()); + EXPECT_TRUE(sd.IsSet()); } TEST_F(SetDecorationTest, ToStr) { diff --git a/src/ast/variable_decoration.cc b/src/ast/variable_decoration.cc index a3e605d1f2..5e902f164d 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::IsBinding() const { - return false; -} - bool VariableDecoration::IsBuiltin() const { return false; } @@ -55,11 +51,6 @@ bool VariableDecoration::IsSet() const { return false; } -BindingDecoration* VariableDecoration::AsBinding() { - assert(IsBinding()); - return static_cast(this); -} - BuiltinDecoration* VariableDecoration::AsBuiltin() { assert(IsBuiltin()); return static_cast(this); diff --git a/src/ast/variable_decoration.h b/src/ast/variable_decoration.h index 7dc1bc8bb7..a6b4f9b24b 100644 --- a/src/ast/variable_decoration.h +++ b/src/ast/variable_decoration.h @@ -25,7 +25,6 @@ namespace tint { namespace ast { -class BindingDecoration; class BuiltinDecoration; class ConstantIdDecoration; class LocationDecoration; @@ -42,8 +41,6 @@ class VariableDecoration : public Castable { /// @return the decoration kind DecorationKind GetKind() const override; - /// @returns true if this is a binding decoration - virtual bool IsBinding() const; /// @returns true if this is a builtin decoration virtual bool IsBuiltin() const; /// @returns true if this is a constant id decoration @@ -53,8 +50,6 @@ class VariableDecoration : public Castable { /// @returns true if this is a set decoration virtual bool IsSet() const; - /// @returns the decoration as a binding decoration - BindingDecoration* AsBinding(); /// @returns the decoration as a builtin decoration BuiltinDecoration* AsBuiltin(); /// @returns the decoration as a constant id decoration 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 4ec18060c0..8e8ddc3d2f 100644 --- a/src/reader/wgsl/parser_impl_global_variable_decl_test.cc +++ b/src/reader/wgsl/parser_impl_global_variable_decl_test.cc @@ -104,7 +104,7 @@ TEST_F(ParserImplTest, GlobalVariableDecl_WithDecoration) { auto& decorations = v->decorations(); ASSERT_EQ(decorations.size(), 2u); - ASSERT_TRUE(decorations[0]->IsBinding()); + ASSERT_TRUE(decorations[0]->Is()); ASSERT_TRUE(decorations[1]->IsSet()); } @@ -138,7 +138,7 @@ TEST_F(ParserImplTest, GlobalVariableDecl_WithDecoration_MulitpleGroups) { auto& decorations = v->decorations(); ASSERT_EQ(decorations.size(), 2u); - ASSERT_TRUE(decorations[0]->IsBinding()); + ASSERT_TRUE(decorations[0]->Is()); ASSERT_TRUE(decorations[1]->IsSet()); } diff --git a/src/reader/wgsl/parser_impl_variable_decoration_test.cc b/src/reader/wgsl/parser_impl_variable_decoration_test.cc index 3126df8b83..33ba645653 100644 --- a/src/reader/wgsl/parser_impl_variable_decoration_test.cc +++ b/src/reader/wgsl/parser_impl_variable_decoration_test.cc @@ -183,9 +183,9 @@ TEST_F(ParserImplTest, VariableDecoration_Binding) { auto* var_deco = deco.value->As(); ASSERT_NE(var_deco, nullptr); ASSERT_FALSE(p->has_error()); - ASSERT_TRUE(var_deco->IsBinding()); + ASSERT_TRUE(var_deco->Is()); - auto* binding = var_deco->AsBinding(); + auto* binding = var_deco->As(); EXPECT_EQ(binding->value(), 4u); } diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc index 8a56827cd2..a4a4840ef2 100644 --- a/src/writer/spirv/builder.cc +++ b/src/writer/spirv/builder.cc @@ -787,10 +787,10 @@ bool Builder::GenerateGlobalVariable(ast::Variable* var) { push_annot(spv::Op::OpDecorate, {Operand::Int(var_id), Operand::Int(SpvDecorationLocation), Operand::Int(deco->AsLocation()->value())}); - } else if (deco->IsBinding()) { + } else if (auto* binding = deco->As()) { push_annot(spv::Op::OpDecorate, {Operand::Int(var_id), Operand::Int(SpvDecorationBinding), - Operand::Int(deco->AsBinding()->value())}); + Operand::Int(binding->value())}); } else if (deco->IsSet()) { push_annot( spv::Op::OpDecorate, diff --git a/src/writer/wgsl/generator_impl.cc b/src/writer/wgsl/generator_impl.cc index f8cd1fc74b..e2dda1cf7c 100644 --- a/src/writer/wgsl/generator_impl.cc +++ b/src/writer/wgsl/generator_impl.cc @@ -641,8 +641,8 @@ bool GeneratorImpl::EmitVariableDecorations(ast::DecoratedVariable* var) { } first = false; - if (deco->IsBinding()) { - out_ << "binding(" << deco->AsBinding()->value() << ")"; + if (auto* binding = deco->As()) { + out_ << "binding(" << binding->value() << ")"; } else if (deco->IsSet()) { out_ << "set(" << deco->AsSet()->value() << ")"; } else if (deco->IsLocation()) {