From 72b6bb94c206b17d1f9e1cce6c6c3c79e31318f4 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Tue, 9 Nov 2021 09:35:00 +0000 Subject: [PATCH] ast: Remove ConstructorExpression ConstructorExpression is abstract, and now only has a single class deriving from it - TypeConstructorExpression. Just use that instead. Bug: tint:888 Change-Id: I7ee52ad645bcd8a8a68710c63a1fdf834b5b2a24 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/68842 Kokoro: Kokoro Reviewed-by: James Price --- src/BUILD.gn | 2 - src/CMakeLists.txt | 2 - src/ast/constructor_expression.cc | 30 --------- src/ast/constructor_expression.h | 41 ------------ src/ast/type_constructor_expression.h | 4 +- .../wgsl/parser_impl_const_expr_test.cc | 4 -- .../parser_impl_primary_expression_test.cc | 3 - .../wgsl/parser_impl_variable_stmt_test.cc | 8 +-- src/resolver/resolver.cc | 66 +++++++++---------- src/resolver/resolver.h | 2 +- src/writer/glsl/generator_impl.cc | 9 +-- src/writer/glsl/generator_impl.h | 6 -- src/writer/hlsl/generator_impl.cc | 9 +-- src/writer/hlsl/generator_impl.h | 6 -- src/writer/msl/generator_impl.cc | 9 +-- src/writer/msl/generator_impl.h | 6 -- src/writer/spirv/builder.cc | 2 +- src/writer/wgsl/generator_impl.cc | 9 +-- src/writer/wgsl/generator_impl.h | 8 +-- 19 files changed, 48 insertions(+), 178 deletions(-) delete mode 100644 src/ast/constructor_expression.cc delete mode 100644 src/ast/constructor_expression.h diff --git a/src/BUILD.gn b/src/BUILD.gn index 2dc31b8abf..db0f44dd9d 100644 --- a/src/BUILD.gn +++ b/src/BUILD.gn @@ -208,8 +208,6 @@ libtint_source_set("libtint_core_all_src") { "ast/call_statement.h", "ast/case_statement.cc", "ast/case_statement.h", - "ast/constructor_expression.cc", - "ast/constructor_expression.h", "ast/continue_statement.cc", "ast/continue_statement.h", "ast/decoration.cc", diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 50090cdb08..d1a41c1b80 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -74,8 +74,6 @@ set(TINT_LIB_SRCS ast/call_statement.h ast/case_statement.cc ast/case_statement.h - ast/constructor_expression.cc - ast/constructor_expression.h ast/continue_statement.cc ast/continue_statement.h ast/decoration.cc diff --git a/src/ast/constructor_expression.cc b/src/ast/constructor_expression.cc deleted file mode 100644 index e1a8224b68..0000000000 --- a/src/ast/constructor_expression.cc +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright 2020 The Tint Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "src/ast/constructor_expression.h" - -TINT_INSTANTIATE_TYPEINFO(tint::ast::ConstructorExpression); - -namespace tint { -namespace ast { - -ConstructorExpression::~ConstructorExpression() = default; - -ConstructorExpression::ConstructorExpression(ConstructorExpression&&) = default; - -ConstructorExpression::ConstructorExpression(ProgramID pid, const Source& src) - : Base(pid, src) {} - -} // namespace ast -} // namespace tint diff --git a/src/ast/constructor_expression.h b/src/ast/constructor_expression.h deleted file mode 100644 index fc19e98ee6..0000000000 --- a/src/ast/constructor_expression.h +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright 2020 The Tint Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef SRC_AST_CONSTRUCTOR_EXPRESSION_H_ -#define SRC_AST_CONSTRUCTOR_EXPRESSION_H_ - -#include "src/ast/expression.h" - -namespace tint { -namespace ast { - -/// Base class for constructor style expressions -class ConstructorExpression - : public Castable { - public: - ~ConstructorExpression() override; - - protected: - /// Constructor - /// @param pid the identifier of the program that owns this node - /// @param src the source of this node - ConstructorExpression(ProgramID pid, const Source& src); - /// Move constructor - ConstructorExpression(ConstructorExpression&&); -}; - -} // namespace ast -} // namespace tint - -#endif // SRC_AST_CONSTRUCTOR_EXPRESSION_H_ diff --git a/src/ast/type_constructor_expression.h b/src/ast/type_constructor_expression.h index 94095e1ca5..d6ed7b5275 100644 --- a/src/ast/type_constructor_expression.h +++ b/src/ast/type_constructor_expression.h @@ -17,7 +17,7 @@ #include -#include "src/ast/constructor_expression.h" +#include "src/ast/expression.h" namespace tint { namespace ast { @@ -27,7 +27,7 @@ class Type; /// A type specific constructor class TypeConstructorExpression - : public Castable { + : public Castable { public: /// Constructor /// @param pid the identifier of the program that owns this node diff --git a/src/reader/wgsl/parser_impl_const_expr_test.cc b/src/reader/wgsl/parser_impl_const_expr_test.cc index 62e5e3ff96..baf5823e48 100644 --- a/src/reader/wgsl/parser_impl_const_expr_test.cc +++ b/src/reader/wgsl/parser_impl_const_expr_test.cc @@ -24,7 +24,6 @@ TEST_F(ParserImplTest, ConstExpr_TypeDecl) { auto e = p->expect_const_expr(); ASSERT_FALSE(p->has_error()) << p->error(); ASSERT_FALSE(e.errored); - ASSERT_TRUE(e->Is()); ASSERT_TRUE(e->Is()); auto* t = e->As(); @@ -46,7 +45,6 @@ TEST_F(ParserImplTest, ConstExpr_TypeDecl_Empty) { auto e = p->expect_const_expr(); ASSERT_FALSE(p->has_error()) << p->error(); ASSERT_FALSE(e.errored); - ASSERT_TRUE(e->Is()); ASSERT_TRUE(e->Is()); auto* t = e->As(); @@ -61,7 +59,6 @@ TEST_F(ParserImplTest, ConstExpr_TypeDecl_TrailingComma) { auto e = p->expect_const_expr(); ASSERT_FALSE(p->has_error()) << p->error(); ASSERT_FALSE(e.errored); - ASSERT_TRUE(e->Is()); ASSERT_TRUE(e->Is()); auto* t = e->As(); @@ -137,7 +134,6 @@ TEST_F(ParserImplTest, ConstExpr_RegisteredType) { auto e = p->expect_const_expr(); ASSERT_FALSE(e.errored); - ASSERT_TRUE(e->Is()); ASSERT_TRUE(e->Is()); } diff --git a/src/reader/wgsl/parser_impl_primary_expression_test.cc b/src/reader/wgsl/parser_impl_primary_expression_test.cc index 1d44f16ae4..e364e5a409 100644 --- a/src/reader/wgsl/parser_impl_primary_expression_test.cc +++ b/src/reader/wgsl/parser_impl_primary_expression_test.cc @@ -39,7 +39,6 @@ TEST_F(ParserImplTest, PrimaryExpression_TypeDecl) { EXPECT_FALSE(e.errored); EXPECT_FALSE(p->has_error()) << p->error(); ASSERT_NE(e.value, nullptr); - ASSERT_TRUE(e->Is()); ASSERT_TRUE(e->Is()); auto* ty = e->As(); @@ -65,7 +64,6 @@ TEST_F(ParserImplTest, PrimaryExpression_TypeDecl_ZeroConstructor) { EXPECT_FALSE(e.errored); EXPECT_FALSE(p->has_error()) << p->error(); ASSERT_NE(e.value, nullptr); - ASSERT_TRUE(e->Is()); ASSERT_TRUE(e->Is()); auto* ty = e->As(); @@ -227,7 +225,6 @@ TEST_F(ParserImplTest, PrimaryExpression_Cast) { EXPECT_FALSE(e.errored); EXPECT_FALSE(p->has_error()) << p->error(); ASSERT_NE(e.value, nullptr); - ASSERT_TRUE(e->Is()); ASSERT_TRUE(e->Is()); auto* c = e->As(); diff --git a/src/reader/wgsl/parser_impl_variable_stmt_test.cc b/src/reader/wgsl/parser_impl_variable_stmt_test.cc index 39e5e62064..5a6653dc4f 100644 --- a/src/reader/wgsl/parser_impl_variable_stmt_test.cc +++ b/src/reader/wgsl/parser_impl_variable_stmt_test.cc @@ -90,7 +90,7 @@ TEST_F(ParserImplTest, VariableStmt_VariableDecl_ArrayInit) { EXPECT_EQ(e->variable->symbol, p->builder().Symbols().Get("a")); ASSERT_NE(e->variable->constructor, nullptr); - EXPECT_TRUE(e->variable->constructor->Is()); + EXPECT_TRUE(e->variable->constructor->Is()); } TEST_F(ParserImplTest, VariableStmt_VariableDecl_ArrayInit_NoSpace) { @@ -105,7 +105,7 @@ TEST_F(ParserImplTest, VariableStmt_VariableDecl_ArrayInit_NoSpace) { EXPECT_EQ(e->variable->symbol, p->builder().Symbols().Get("a")); ASSERT_NE(e->variable->constructor, nullptr); - EXPECT_TRUE(e->variable->constructor->Is()); + EXPECT_TRUE(e->variable->constructor->Is()); } TEST_F(ParserImplTest, VariableStmt_VariableDecl_VecInit) { @@ -120,7 +120,7 @@ TEST_F(ParserImplTest, VariableStmt_VariableDecl_VecInit) { EXPECT_EQ(e->variable->symbol, p->builder().Symbols().Get("a")); ASSERT_NE(e->variable->constructor, nullptr); - EXPECT_TRUE(e->variable->constructor->Is()); + EXPECT_TRUE(e->variable->constructor->Is()); } TEST_F(ParserImplTest, VariableStmt_VariableDecl_VecInit_NoSpace) { @@ -135,7 +135,7 @@ TEST_F(ParserImplTest, VariableStmt_VariableDecl_VecInit_NoSpace) { EXPECT_EQ(e->variable->symbol, p->builder().Symbols().Get("a")); ASSERT_NE(e->variable->constructor, nullptr); - EXPECT_TRUE(e->variable->constructor->Is()); + EXPECT_TRUE(e->variable->constructor->Is()); } TEST_F(ParserImplTest, VariableStmt_Let) { diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc index 69433543d2..25bcd445e5 100644 --- a/src/resolver/resolver.cc +++ b/src/resolver/resolver.cc @@ -2362,8 +2362,8 @@ sem::Expression* Resolver::Expression(const ast::Expression* root) { sem_expr = Bitcast(bitcast); } else if (auto* call = expr->As()) { sem_expr = Call(call); - } else if (auto* ctor = expr->As()) { - sem_expr = Constructor(ctor); + } else if (auto* ctor = expr->As()) { + sem_expr = TypeConstructor(ctor); } else if (auto* ident = expr->As()) { sem_expr = Identifier(ident); } else if (auto* literal = expr->As()) { @@ -2730,41 +2730,37 @@ bool Resolver::ValidateFunctionCall(const sem::Call* call) { return true; } -sem::Expression* Resolver::Constructor(const ast::ConstructorExpression* expr) { - if (auto* type_ctor = expr->As()) { - auto* ty = Type(type_ctor->type); - if (!ty) { - return nullptr; - } - - // Now that the argument types have been determined, make sure that they - // obey the constructor type rules laid out in - // https://gpuweb.github.io/gpuweb/wgsl.html#type-constructor-expr. - bool ok = true; - if (auto* vec_type = ty->As()) { - ok = ValidateVectorConstructor(type_ctor, vec_type); - } else if (auto* mat_type = ty->As()) { - ok = ValidateMatrixConstructor(type_ctor, mat_type); - } else if (ty->is_scalar()) { - ok = ValidateScalarConstructor(type_ctor, ty); - } else if (auto* arr_type = ty->As()) { - ok = ValidateArrayConstructor(type_ctor, arr_type); - } else if (auto* struct_type = ty->As()) { - ok = ValidateStructureConstructor(type_ctor, struct_type); - } else { - AddError("type is not constructible", type_ctor->source); - return nullptr; - } - if (!ok) { - return nullptr; - } - - auto val = EvaluateConstantValue(expr, ty); - return builder_->create(expr, ty, current_statement_, val); +sem::Expression* Resolver::TypeConstructor( + const ast::TypeConstructorExpression* expr) { + auto* ty = Type(expr->type); + if (!ty) { + return nullptr; } - TINT_ICE(Resolver, diagnostics_) << "unexpected constructor expression type"; - return nullptr; + // Now that the argument types have been determined, make sure that they + // obey the constructor type rules laid out in + // https://gpuweb.github.io/gpuweb/wgsl.html#type-constructor-expr. + bool ok = true; + if (auto* vec_type = ty->As()) { + ok = ValidateVectorConstructor(expr, vec_type); + } else if (auto* mat_type = ty->As()) { + ok = ValidateMatrixConstructor(expr, mat_type); + } else if (ty->is_scalar()) { + ok = ValidateScalarConstructor(expr, ty); + } else if (auto* arr_type = ty->As()) { + ok = ValidateArrayConstructor(expr, arr_type); + } else if (auto* struct_type = ty->As()) { + ok = ValidateStructureConstructor(expr, struct_type); + } else { + AddError("type is not constructible", expr->source); + return nullptr; + } + if (!ok) { + return nullptr; + } + + auto val = EvaluateConstantValue(expr, ty); + return builder_->create(expr, ty, current_statement_, val); } sem::Expression* Resolver::Literal(const ast::Literal* literal) { diff --git a/src/resolver/resolver.h b/src/resolver/resolver.h index c19dfeec59..b6c511866c 100644 --- a/src/resolver/resolver.h +++ b/src/resolver/resolver.h @@ -171,7 +171,6 @@ class Resolver { sem::Expression* Binary(const ast::BinaryExpression*); sem::Expression* Bitcast(const ast::BitcastExpression*); sem::Expression* Call(const ast::CallExpression*); - sem::Expression* Constructor(const ast::ConstructorExpression*); sem::Expression* Expression(const ast::Expression*); sem::Function* Function(const ast::Function*); sem::Call* FunctionCall(const ast::CallExpression*); @@ -179,6 +178,7 @@ class Resolver { sem::Call* IntrinsicCall(const ast::CallExpression*, sem::IntrinsicType); sem::Expression* Literal(const ast::Literal*); sem::Expression* MemberAccessor(const ast::MemberAccessorExpression*); + sem::Expression* TypeConstructor(const ast::TypeConstructorExpression*); sem::Expression* UnaryOp(const ast::UnaryOpExpression*); // Statement resolving methods diff --git a/src/writer/glsl/generator_impl.cc b/src/writer/glsl/generator_impl.cc index 1d0213a19e..7f26088c6b 100644 --- a/src/writer/glsl/generator_impl.cc +++ b/src/writer/glsl/generator_impl.cc @@ -1385,11 +1385,6 @@ bool GeneratorImpl::EmitCase(const ast::CaseStatement* stmt) { return true; } -bool GeneratorImpl::EmitConstructor(std::ostream& out, - const ast::ConstructorExpression* expr) { - return EmitTypeConstructor(out, expr->As()); -} - bool GeneratorImpl::EmitTypeConstructor( std::ostream& out, const ast::TypeConstructorExpression* expr) { @@ -1471,8 +1466,8 @@ bool GeneratorImpl::EmitExpression(std::ostream& out, if (auto* c = expr->As()) { return EmitCall(out, c); } - if (auto* c = expr->As()) { - return EmitConstructor(out, c); + if (auto* c = expr->As()) { + return EmitTypeConstructor(out, c); } if (auto* i = expr->As()) { return EmitIdentifier(out, i); diff --git a/src/writer/glsl/generator_impl.h b/src/writer/glsl/generator_impl.h index c478d3b32d..8d6a4f5fc2 100644 --- a/src/writer/glsl/generator_impl.h +++ b/src/writer/glsl/generator_impl.h @@ -188,12 +188,6 @@ class GeneratorImpl : public TextGenerator { /// @param stmt the statement /// @returns true if the statement was emitted successfully bool EmitCase(const ast::CaseStatement* stmt); - /// Handles generating constructor expressions - /// @param out the output of the expression stream - /// @param expr the constructor expression - /// @returns true if the expression was emitted - bool EmitConstructor(std::ostream& out, - const ast::ConstructorExpression* expr); /// Handles generating a discard statement /// @param stmt the discard statement /// @returns true if the statement was successfully emitted diff --git a/src/writer/hlsl/generator_impl.cc b/src/writer/hlsl/generator_impl.cc index 8a2d114058..92fe82e391 100644 --- a/src/writer/hlsl/generator_impl.cc +++ b/src/writer/hlsl/generator_impl.cc @@ -2112,11 +2112,6 @@ bool GeneratorImpl::EmitCase(const ast::SwitchStatement* s, size_t case_idx) { return true; } -bool GeneratorImpl::EmitConstructor(std::ostream& out, - const ast::ConstructorExpression* expr) { - return EmitTypeConstructor(out, expr->As()); -} - bool GeneratorImpl::EmitTypeConstructor( std::ostream& out, const ast::TypeConstructorExpression* expr) { @@ -2203,8 +2198,8 @@ bool GeneratorImpl::EmitExpression(std::ostream& out, if (auto* c = expr->As()) { return EmitCall(out, c); } - if (auto* c = expr->As()) { - return EmitConstructor(out, c); + if (auto* c = expr->As()) { + return EmitTypeConstructor(out, c); } if (auto* i = expr->As()) { return EmitIdentifier(out, i); diff --git a/src/writer/hlsl/generator_impl.h b/src/writer/hlsl/generator_impl.h index 1aadf4aac1..1945d4e6ba 100644 --- a/src/writer/hlsl/generator_impl.h +++ b/src/writer/hlsl/generator_impl.h @@ -217,12 +217,6 @@ class GeneratorImpl : public TextGenerator { /// @param case_idx the index of the switch case in the switch statement /// @returns true if the statement was emitted successfully bool EmitCase(const ast::SwitchStatement* s, size_t case_idx); - /// Handles generating constructor expressions - /// @param out the output of the expression stream - /// @param expr the constructor expression - /// @returns true if the expression was emitted - bool EmitConstructor(std::ostream& out, - const ast::ConstructorExpression* expr); /// Handles generating a discard statement /// @param stmt the discard statement /// @returns true if the statement was successfully emitted diff --git a/src/writer/msl/generator_impl.cc b/src/writer/msl/generator_impl.cc index f733b2707e..a981b8aaf1 100644 --- a/src/writer/msl/generator_impl.cc +++ b/src/writer/msl/generator_impl.cc @@ -1291,11 +1291,6 @@ bool GeneratorImpl::EmitCase(const ast::CaseStatement* stmt) { return true; } -bool GeneratorImpl::EmitConstructor(std::ostream& out, - const ast::ConstructorExpression* expr) { - return EmitTypeConstructor(out, expr->As()); -} - bool GeneratorImpl::EmitContinue(const ast::ContinueStatement*) { if (!emit_continuing_()) { return false; @@ -1430,8 +1425,8 @@ bool GeneratorImpl::EmitExpression(std::ostream& out, if (auto* c = expr->As()) { return EmitCall(out, c); } - if (auto* c = expr->As()) { - return EmitConstructor(out, c); + if (auto* c = expr->As()) { + return EmitTypeConstructor(out, c); } if (auto* i = expr->As()) { return EmitIdentifier(out, i); diff --git a/src/writer/msl/generator_impl.h b/src/writer/msl/generator_impl.h index 557c9414f7..58cfe386fb 100644 --- a/src/writer/msl/generator_impl.h +++ b/src/writer/msl/generator_impl.h @@ -182,12 +182,6 @@ class GeneratorImpl : public TextGenerator { /// @param stmt the statement /// @returns true if the statement was emitted successfully bool EmitCase(const ast::CaseStatement* stmt); - /// Handles generating constructor expressions - /// @param out the output of the expression stream - /// @param expr the constructor expression - /// @returns true if the expression was emitted - bool EmitConstructor(std::ostream& out, - const ast::ConstructorExpression* expr); /// Handles a continue statement /// @param stmt the statement to emit /// @returns true if the statement was emitted successfully diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc index 75d4d0d770..4d063779b2 100644 --- a/src/writer/spirv/builder.cc +++ b/src/writer/spirv/builder.cc @@ -577,7 +577,7 @@ uint32_t Builder::GenerateExpression(const ast::Expression* expr) { if (auto* c = expr->As()) { return GenerateCallExpression(c); } - if (auto* c = expr->As()) { + if (auto* c = expr->As()) { return GenerateConstructorExpression(nullptr, c); } if (auto* i = expr->As()) { diff --git a/src/writer/wgsl/generator_impl.cc b/src/writer/wgsl/generator_impl.cc index 97ff2ffcb4..cf4bd4cfdb 100644 --- a/src/writer/wgsl/generator_impl.cc +++ b/src/writer/wgsl/generator_impl.cc @@ -134,8 +134,8 @@ bool GeneratorImpl::EmitExpression(std::ostream& out, if (auto* l = expr->As()) { return EmitLiteral(out, l); } - if (auto* c = expr->As()) { - return EmitConstructor(out, c); + if (auto* c = expr->As()) { + return EmitTypeConstructor(out, c); } if (auto* m = expr->As()) { return EmitMemberAccessor(out, m); @@ -243,11 +243,6 @@ bool GeneratorImpl::EmitCall(std::ostream& out, return true; } -bool GeneratorImpl::EmitConstructor(std::ostream& out, - const ast::ConstructorExpression* expr) { - return EmitTypeConstructor(out, expr->As()); -} - bool GeneratorImpl::EmitTypeConstructor( std::ostream& out, const ast::TypeConstructorExpression* expr) { diff --git a/src/writer/wgsl/generator_impl.h b/src/writer/wgsl/generator_impl.h index 5a8559e120..b1d83b3214 100644 --- a/src/writer/wgsl/generator_impl.h +++ b/src/writer/wgsl/generator_impl.h @@ -120,18 +120,12 @@ class GeneratorImpl : public TextGenerator { /// Handles generating an identifier expression /// @param out the output of the expression stream /// @param expr the identifier expression - /// @returns true if the identifeir was emitted + /// @returns true if the identifier was emitted bool EmitIdentifier(std::ostream& out, const ast::IdentifierExpression* expr); /// Handles an if statement /// @param stmt the statement to emit /// @returns true if the statement was successfully emitted bool EmitIf(const ast::IfStatement* stmt); - /// Handles generating constructor expressions - /// @param out the output of the expression stream - /// @param expr the constructor expression - /// @returns true if the expression was emitted - bool EmitConstructor(std::ostream& out, - const ast::ConstructorExpression* expr); /// Handles generating a discard statement /// @param stmt the discard statement /// @returns true if the statement was successfully emitted