From ad057a42fa2447c099176e710e4a59597f9bec54 Mon Sep 17 00:00:00 2001 From: peter klausler Date: Wed, 20 Mar 2019 11:38:45 -0700 Subject: [PATCH] [flang] Remove OwningPointer, use unique_ptr better instead. Original-commit: flang-compiler/f18@89aff868aaec0a9ab6c65e86458a3e23a93da7d1 Reviewed-on: https://github.com/flang-compiler/f18/pull/346 Tree-same-pre-rewrite: false --- flang/CMakeLists.txt | 4 +- flang/documentation/C++style.md | 17 +- flang/lib/FIR/afforestation.cc | 10 +- flang/lib/common/idioms.h | 1 - flang/lib/common/indirection.h | 186 ++++----------------- flang/lib/common/unwrap.h | 15 +- flang/lib/evaluate/call.h | 4 +- flang/lib/evaluate/characteristics.cc | 5 +- flang/lib/evaluate/characteristics.h | 2 +- flang/lib/evaluate/common.h | 13 +- flang/lib/evaluate/constant.h | 4 +- flang/lib/evaluate/descender.h | 6 +- flang/lib/evaluate/expression.cc | 32 +++- flang/lib/evaluate/expression.h | 33 ++-- flang/lib/evaluate/fold.cc | 2 +- flang/lib/evaluate/type.h | 2 +- flang/lib/evaluate/variable.cc | 4 +- flang/lib/evaluate/variable.h | 6 +- flang/lib/parser/parse-tree.cc | 9 + flang/lib/parser/parse-tree.h | 23 ++- flang/lib/semantics/assignment.h | 15 +- flang/lib/semantics/check-do-concurrent.cc | 14 +- flang/lib/semantics/check-do-concurrent.h | 8 +- flang/tools/f18/dump.cc | 4 +- flang/tools/f18/stub-evaluate.cc | 8 +- 25 files changed, 167 insertions(+), 260 deletions(-) diff --git a/flang/CMakeLists.txt b/flang/CMakeLists.txt index 4c0f50ee0213..b51177d7d95b 100644 --- a/flang/CMakeLists.txt +++ b/flang/CMakeLists.txt @@ -92,9 +92,9 @@ if(CMAKE_COMPILER_IS_GNUCXX OR (CMAKE_CXX_COMPILER_ID MATCHES "Clang")) endif() set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++17") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -pedantic") - set(CMAKE_CXX_FLAGS_RELEASE "-O2 -DDEBUG") + set(CMAKE_CXX_FLAGS_RELEASE "-O2") set(CMAKE_CXX_FLAGS_MINSIZEREL "-O2 '-DCHECK=(void)'") - set(CMAKE_CXX_FLAGS_DEBUG "-g -DDEBUG") + set(CMAKE_CXX_FLAGS_DEBUG "-g -DDEBUGF18") # Building shared libraries is death on performance with GCC by default # due to the need to preserve the right to override external entry points diff --git a/flang/documentation/C++style.md b/flang/documentation/C++style.md index ce846a721267..7c5b7a2711d4 100644 --- a/flang/documentation/C++style.md +++ b/flang/documentation/C++style.md @@ -192,20 +192,17 @@ will be defined to return a reference.) wherever appropriate. * `std::unique_ptr<>`: A nullable pointer with ownership, null by default, not copyable, reassignable. +F18 has a helpful `Deleter<>` class template that makes `unique_ptr<>` +easier to use with forward-referenced data types. * `std::shared_ptr<>`: A nullable pointer with shared ownership via reference counting, null by default, shallowly copyable, reassignable, and slow. -* `OwningPointer<>`: A nullable pointer with ownership, better suited -for use with forward-defined types than `std::unique_ptr<>` is. -Null by default, optionally copyable, reassignable. -Does not have direct means for allocating data, and inconveniently requires -the definition of an external destructor. * `Indirection<>`: A non-nullable pointer with ownership and optional deep copy semantics; reassignable. Often better than a reference (due to ownership) or `std::unique_ptr<>` (due to non-nullability and copyability). Can be wrapped in `std::optional<>` when nullability is required. -* `ForwardReference<>`: A non-nullable `OwningPointer<>`, or a variant of -`Indirection<>` that works with forward-declared content types; it's both. +Usable with forward-referenced data types with some use of `extern template` +in headers and explicit template instantiation in source files. * `CountedReference<>`: A nullable pointer with shared ownership via reference counting, null by default, shallowly copyable, reassignable. Safe to use *only* when the data are private to just one @@ -219,11 +216,9 @@ A feature matrix: | ------- | -------- | ------------ | ------ | ------------ | -------- | ------------------ | | `*p` | yes | no | no | yes | shallowly | yes | | `&r` | no | n/a | no | no | shallowly | yes | -| `unique_ptr<>` | yes | yes | yes | yes | no | no | +| `unique_ptr<>` | yes | yes | yes | yes | no | yes, with work | | `shared_ptr<>` | yes | yes | yes | yes | shallowly | no | -| `OwningPointer<>` | yes | yes | yes | yes | optionally deeply | yes | -| `Indirection<>` | no | n/a | yes | yes | optionally deeply | no | -| `ForwardReference<>` | no | n/a | yes | yes | optionally deeply | yes | +| `Indirection<>` | no | n/a | yes | yes | optionally deeply | yes, with work | | `CountedReference<>` | yes | yes | yes | yes | shallowly | no | ### Overall design preferences diff --git a/flang/lib/FIR/afforestation.cc b/flang/lib/FIR/afforestation.cc index 3be8f7e71fac..242a16b95b00 100644 --- a/flang/lib/FIR/afforestation.cc +++ b/flang/lib/FIR/afforestation.cc @@ -25,9 +25,9 @@ namespace Fortran::FIR { namespace { -Expression *ExprRef(const parser::Expr &a) { return &a.typedExpr.value(); } +Expression *ExprRef(const parser::Expr &a) { return a.typedExpr.get(); } Expression *ExprRef(const common::Indirection &a) { - return &a.value().typedExpr.value(); + return a.value().typedExpr.get(); } struct LinearOp; @@ -1296,9 +1296,9 @@ public: const std::vector &refs) { auto &cases{ std::get>(caseConstruct->t)}; - SwitchCaseArguments result{ - GetSwitchCaseSelector(caseConstruct), unspecifiedLabel, - populateSwitchValues(builder_, cases), std::move(refs)}; + SwitchCaseArguments result{GetSwitchCaseSelector(caseConstruct), + unspecifiedLabel, populateSwitchValues(builder_, cases), + std::move(refs)}; cleanupSwitchPairs( result.defLab, result.values, result.labels); return result; diff --git a/flang/lib/common/idioms.h b/flang/lib/common/idioms.h index 524c94f3bf5e..747125ee40f3 100644 --- a/flang/lib/common/idioms.h +++ b/flang/lib/common/idioms.h @@ -135,7 +135,6 @@ template struct ListItemCount { } // Given a const reference to a value, return a copy of the value. - template A Clone(const A &x) { return x; } } #endif // FORTRAN_COMMON_IDIOMS_H_ diff --git a/flang/lib/common/indirection.h b/flang/lib/common/indirection.h index c35f10f6f87e..5ddc2ef76ffd 100644 --- a/flang/lib/common/indirection.h +++ b/flang/lib/common/indirection.h @@ -15,20 +15,16 @@ #ifndef FORTRAN_COMMON_INDIRECTION_H_ #define FORTRAN_COMMON_INDIRECTION_H_ -// Defines several smart pointer class templates that are rather like -// std::unique_ptr<>. -// - Indirection<> is, like a C++ reference type, restricted to be non-null -// when constructed or assigned. -// - OwningPointer<> is like a std::unique_ptr<> with an out-of-line destructor. -// This makes it suitable for use with forward-declared content types -// in a way that bare C pointers allow but std::unique_ptr<> cannot. -// - ForwardReference<> is a kind of Indirection<> that, like OwningPointer<>, -// accommodates the use of forward declarations. -// Users of Indirection<> and ForwardReference<> need to check whether their -// pointers are null. Like a C++ reference, they are meant to be as invisible -// as possible. -// All of these can optionally support copy construction -// and copy assignment. +// Define a smart pointer class template that is rather like +// non-nullable std::unique_ptr<>. Indirection<> is, like a C++ reference +// type, restricted to be non-null when constructed or assigned. +// Indirection<> optionally supports copy construction and copy assignment. +// +// To use Indirection<> with forward-referenced types, add +// extern template class Fortran::common::Indirection; +// outside any namespace in a header before use, and +// template class Fortran::common::Indirection; +// in one C++ source file later where a definition of the type is visible. #include "../common/idioms.h" #include @@ -66,8 +62,10 @@ public: A &value() { return *p_; } const A &value() const { return *p_; } - bool operator==(const A &x) const { return *p_ == x; } + bool operator==(const A &that) const { return *p_ == that; } bool operator==(const Indirection &that) const { return *p_ == *that.p_; } + bool operator!=(const A &that) const { return *p_ != that; } + bool operator!=(const Indirection &that) const { return *p_ != *that.p_; } template static Indirection Make(ARGS &&... args) { return {new A(std::forward(args)...)}; @@ -117,8 +115,10 @@ public: A &value() { return *p_; } const A &value() const { return *p_; } - bool operator==(const A &x) const { return *p_ == x; } + bool operator==(const A &that) const { return *p_ == that; } bool operator==(const Indirection &that) const { return *p_ == *that.p_; } + bool operator!=(const A &that) const { return *p_ != that; } + bool operator!=(const Indirection &that) const { return *p_ != *that.p_; } template static Indirection Make(ARGS &&... args) { return {new A(std::forward(args)...)}; @@ -128,148 +128,22 @@ private: A *p_{nullptr}; }; -// A variant of Indirection suitable for use with forward-referenced types. -// These are nullable pointers, not references. Allocation is not available, -// and a single externalized destructor must be defined. Copyable if an -// external copy constructor and operator= are implemented. -template class OwningPointer { +template using CopyableIndirection = Indirection; + +// For use with std::unique_ptr<> when declaring owning pointers to +// forward-referenced types, here's a minimal custom deleter that avoids +// some of the drama with std::default_delete<>. Invoke DEFINE_DELETER() +// later in exactly one C++ source file where a complete definition of the +// type is visible. Be advised, std::unique_ptr<> does not have copy +// semantics; if you need ownership, copy semantics, and nullability, +// std::optional> works. +template class Deleter { public: - using element_type = A; - - OwningPointer() {} - OwningPointer(OwningPointer &&that) : p_{that.p_} { that.p_ = nullptr; } - explicit OwningPointer(std::unique_ptr &&that) : p_{that.release()} {} - explicit OwningPointer(A *&&p) : p_{p} { p = nullptr; } - - // Must be externally defined; see DEFINE_OWNING_DESTRUCTOR below - ~OwningPointer(); - - // Must be externally defined if copying is needed. - OwningPointer(const A &); - OwningPointer(const OwningPointer &); - OwningPointer &operator=(const A &); - OwningPointer &operator=(const OwningPointer &); - - OwningPointer &operator=(OwningPointer &&that) { - auto tmp{p_}; - p_ = that.p_; - that.p_ = tmp; - return *this; - } - OwningPointer &operator=(A *&&p) { - return *this = OwningPointer(std::move(p)); - } - - bool has_value() const { return p_ != nullptr; } - A &value() { - CHECK(p_ != nullptr); - return *p_; - } - const A &value() const { - CHECK(p_ != nullptr); - return *p_; - } - - bool operator==(const A &x) const { return p_ != nullptr && *p_ == x; } - bool operator==(const OwningPointer &that) const { - return (p_ == nullptr && that.p_ == nullptr) || - (that.p_ != nullptr && *this == *that.p_); - } - -private: - A *p_{nullptr}; -}; - -// ForwardReference can be viewed as either a non-nullable variant of -// OwningPointer or as a variant of Indirection that accommodates use with -// a forward-declared content type. -template class ForwardReference { -public: - using element_type = A; - - explicit ForwardReference(std::unique_ptr &&that) : p_{that.release()} {} - explicit ForwardReference(A *&&p) : p_{p} { - CHECK(p_ && "assigning null pointer to ForwardReference"); - p = nullptr; - } - ForwardReference(ForwardReference &&that) : p_{that.p_} { - CHECK(p_ && - "move construction of ForwardReference from null ForwardReference"); - that.p_ = nullptr; - } - - // Must be externally defined; see DEFINE_OWNING_DESTRUCTOR below - ~ForwardReference(); - - // Must be externally defined if copying is needed. - ForwardReference(const A &); - ForwardReference(const ForwardReference &); - ForwardReference &operator=(const A &); - ForwardReference &operator=(const ForwardReference &); - - ForwardReference &operator=(ForwardReference &&that) { - CHECK(that.p_ && - "move assignment of null ForwardReference to ForwardReference"); - auto tmp{p_}; - p_ = that.p_; - that.p_ = tmp; - return *this; - } - ForwardReference &operator=(A *&&p) { - return *this = ForwardReference(std::move(p)); - } - - A &value() { return *p_; } - const A &value() const { return *p_; } - - bool operator==(const A &x) const { return *p_ == x; } - bool operator==(const ForwardReference &that) const { - return *p_ == *that.p_; - } - -private: - A *p_{nullptr}; + void operator()(A *) const; }; } - -// Mandatory instantiation and definition -- put somewhere, not in a namespace -// CLASS here is OwningPointer or ForwardReference. -#define DEFINE_OWNING_DESTRUCTOR(CLASS, A) \ - namespace Fortran::common { \ - template class CLASS; \ - template<> CLASS::~CLASS() { \ - delete p_; \ - p_ = nullptr; \ - } \ +#define DEFINE_DELETER(A) \ + template<> void Fortran::common::Deleter::operator()(A *p) const { \ + delete p; \ } - -// Optional definitions for OwningPointer and ForwardReference -#define DEFINE_OWNING_COPY_CONSTRUCTORS(CLASS, A) \ - namespace Fortran::common { \ - template<> CLASS::CLASS(const A &that) : p_{new A(that)} {} \ - template<> \ - CLASS::CLASS(const CLASS &that) \ - : p_{that.p_ ? new A(*that.p_) : nullptr} {} \ - } -#define DEFINE_OWNING_COPY_ASSIGNMENTS(CLASS, A) \ - namespace Fortran::common { \ - template<> CLASS &CLASS::operator=(const A &that) { \ - delete p_; \ - p_ = new A(that); \ - return *this; \ - } \ - template<> CLASS &CLASS::operator=(const CLASS &that) { \ - delete p_; \ - p_ = that.p_ ? new A(*that.p_) : nullptr; \ - return *this; \ - } \ - } - -#define DEFINE_OWNING_COPY_FUNCTIONS(CLASS, A) \ - DEFINE_OWNING_COPY_CONSTRUCTORS(CLASS, A) \ - DEFINE_OWNING_COPY_ASSIGNMENTS(CLASS, A) -#define DEFINE_OWNING_SPECIAL_FUNCTIONS(CLASS, A) \ - DEFINE_OWNING_DESTRUCTOR(CLASS, A) \ - DEFINE_OWNING_COPY_FUNCTIONS(CLASS, A) - #endif // FORTRAN_COMMON_INDIRECTION_H_ diff --git a/flang/lib/common/unwrap.h b/flang/lib/common/unwrap.h index 4ac8591f6e2f..d94be84a7523 100644 --- a/flang/lib/common/unwrap.h +++ b/flang/lib/common/unwrap.h @@ -1,4 +1,4 @@ -// Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved. +// Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -70,10 +70,6 @@ template auto Unwrap(const std::variant &) -> std::add_const_t *; template auto Unwrap(const Indirection &) -> Constify *; -template -auto Unwrap(const OwningPointer &) -> Constify *; -template -auto Unwrap(const CountedReference &) -> Constify *; // Implementations of specializations template auto Unwrap(B *p) -> Constify * { @@ -144,15 +140,6 @@ auto Unwrap(const Indirection &p) -> Constify * { return Unwrap(*p); } -template -auto Unwrap(const OwningPointer &p) -> Constify * { - if (p.get() != nullptr) { - return Unwrap(*p); - } else { - return nullptr; - } -} - template auto Unwrap(const CountedReference &p) -> Constify * { if (p.get() != nullptr) { diff --git a/flang/lib/evaluate/call.h b/flang/lib/evaluate/call.h index b0b39f87cc4d..0ae8565cd5cc 100644 --- a/flang/lib/evaluate/call.h +++ b/flang/lib/evaluate/call.h @@ -34,7 +34,7 @@ namespace Fortran::evaluate { class ActualArgument { public: explicit ActualArgument(Expr &&x) : value_{std::move(x)} {} - explicit ActualArgument(CopyableIndirection> &&v) + explicit ActualArgument(common::CopyableIndirection> &&v) : value_{std::move(v)} {} Expr &value() { return value_.value(); } @@ -57,7 +57,7 @@ private: // e.g. between X and (X). The parser attempts to parse each argument // first as a variable, then as an expression, and the distinction appears // in the parse tree. - CopyableIndirection> value_; + common::CopyableIndirection> value_; }; using ActualArguments = std::vector>; diff --git a/flang/lib/evaluate/characteristics.cc b/flang/lib/evaluate/characteristics.cc index 79dad84d92c5..284012affbae 100644 --- a/flang/lib/evaluate/characteristics.cc +++ b/flang/lib/evaluate/characteristics.cc @@ -99,7 +99,4 @@ std::ostream &Procedure::Dump(std::ostream &o) const { return o << (sep == '(' ? "()" : ")"); } } - -// Define OwningPointer special member functions -DEFINE_OWNING_SPECIAL_FUNCTIONS( - OwningPointer, evaluate::characteristics::Procedure) +DEFINE_DELETER(Fortran::evaluate::characteristics::Procedure) diff --git a/flang/lib/evaluate/characteristics.h b/flang/lib/evaluate/characteristics.h index d1389e1f696e..14188a56f951 100644 --- a/flang/lib/evaluate/characteristics.h +++ b/flang/lib/evaluate/characteristics.h @@ -54,7 +54,7 @@ struct DummyDataObject { struct DummyProcedure { ENUM_CLASS(Attr, Pointer, Optional) DEFAULT_CONSTRUCTORS_AND_ASSIGNMENTS(DummyProcedure) - common::OwningPointer explicitProcedure; + std::unique_ptr> explicitProcedure; common::EnumSet attrs; bool operator==(const DummyProcedure &) const; std::ostream &Dump(std::ostream &) const; diff --git a/flang/lib/evaluate/common.h b/flang/lib/evaluate/common.h index 35878698ba78..71ca16099054 100644 --- a/flang/lib/evaluate/common.h +++ b/flang/lib/evaluate/common.h @@ -166,11 +166,21 @@ using HostUnsignedInt = // - There are full copy and move semantics for construction and assignment. // - Discriminated unions have a std::variant<> member "u" and support // explicit copy and move constructors as well as comparison for equality. +#define DECLARE_CONSTRUCTORS_AND_ASSIGNMENTS(t) \ + t(const t &); \ + t(t &&); \ + t &operator=(const t &); \ + t &operator=(t &&); #define DEFAULT_CONSTRUCTORS_AND_ASSIGNMENTS(t) \ t(const t &) = default; \ t(t &&) = default; \ t &operator=(const t &) = default; \ t &operator=(t &&) = default; +#define DEFINE_DEFAULT_CONSTRUCTORS_AND_ASSIGNMENTS(t) \ + t::t(const t &) = default; \ + t::t(t &&) = default; \ + t &t::operator=(const t &) = default; \ + t &t::operator=(t &&) = default; #define CLASS_BOILERPLATE(t) \ t() = delete; \ @@ -184,9 +194,6 @@ using HostUnsignedInt = : u(std::move(x)) {} \ bool operator==(const t &that) const { return u == that.u; } -// Force availability of copy construction and assignment -template using CopyableIndirection = common::Indirection; - // Forward definition of Expr<> so that it can be indirectly used in its own // definition template class Expr; diff --git a/flang/lib/evaluate/constant.h b/flang/lib/evaluate/constant.h index aa81ea150a69..cb3396771ea9 100644 --- a/flang/lib/evaluate/constant.h +++ b/flang/lib/evaluate/constant.h @@ -128,8 +128,8 @@ private: std::vector shape_; }; -using StructureConstructorValues = - std::map>>; +using StructureConstructorValues = std::map>>; template<> class Constant diff --git a/flang/lib/evaluate/descender.h b/flang/lib/evaluate/descender.h index 4dea229ff14b..d4d8c255a13a 100644 --- a/flang/lib/evaluate/descender.h +++ b/flang/lib/evaluate/descender.h @@ -51,10 +51,12 @@ public: } } - template void Descend(const CopyableIndirection &p) { + template + void Descend(const common::Indirection &p) { Visit(p.value()); } - template void Descend(CopyableIndirection &p) { + template + void Descend(common::Indirection &p) { Visit(p.value()); } diff --git a/flang/lib/evaluate/expression.cc b/flang/lib/evaluate/expression.cc index 0f4f068be561..d286a707e42b 100644 --- a/flang/lib/evaluate/expression.cc +++ b/flang/lib/evaluate/expression.cc @@ -86,7 +86,8 @@ std::ostream &LogicalOperation::Infix(std::ostream &o) const { } template -std::ostream &Emit(std::ostream &o, const CopyableIndirection> &expr) { +std::ostream &Emit( + std::ostream &o, const common::CopyableIndirection> &expr) { return expr.value().AsFortran(o); } @@ -147,7 +148,7 @@ std::ostream &ExpressionBase::AsFortran(std::ostream &o) const { o << "z'" << x.Hexadecimal() << "'"; }, [&](const NullPointer &) { o << "NULL()"; }, - [&](const CopyableIndirection &s) { + [&](const common::CopyableIndirection &s) { s.value().AsFortran(o); }, [&](const ImpliedDoIndex &i) { o << i.name.ToString(); }, @@ -184,7 +185,7 @@ Expr Expr>::LEN() const { u); } -Expr::~Expr() {} +Expr::~Expr() = default; #if defined(__APPLE__) && defined(__GNUC__) template @@ -312,10 +313,28 @@ std::ostream &DerivedTypeSpecAsFortran( return o; } +GenericExprWrapper::~GenericExprWrapper() = default; + bool GenericExprWrapper::operator==(const GenericExprWrapper &that) const { return v == that.v; } +template int Expr>::GetKind() const { + return std::visit( + [](const auto &kx) { return std::decay_t::Result::kind; }, + u); +} + +int Expr::GetKind() const { + return std::visit( + [](const auto &kx) { return std::decay_t::Result::kind; }, + u); +} + +Expr Expr::LEN() const { + return std::visit([](const auto &kx) { return kx.LEN(); }, u); +} + // Template instantiations to resolve the "extern template" declarations // that appear in expression.h. @@ -329,9 +348,4 @@ FOR_EACH_TYPE_AND_KIND(template class ExpressionBase) FOR_EACH_INTRINSIC_KIND(template class ArrayConstructorValues) FOR_EACH_INTRINSIC_KIND(template class ArrayConstructor) } - -// For reclamation of analyzed expressions to which owning pointers have -// been embedded in the parse tree. This destructor appears here, where -// definitions for all the necessary types are available, to obviate a -// need to include lib/evaluate/*.h headers in the parser proper. -DEFINE_OWNING_SPECIAL_FUNCTIONS(OwningPointer, evaluate::GenericExprWrapper) +DEFINE_DELETER(Fortran::evaluate::GenericExprWrapper) diff --git a/flang/lib/evaluate/expression.h b/flang/lib/evaluate/expression.h index b904e72c7c3a..002e04ddaf99 100644 --- a/flang/lib/evaluate/expression.h +++ b/flang/lib/evaluate/expression.h @@ -124,9 +124,9 @@ public: // Unary operations wrap a single Expr with a CopyableIndirection. // Binary operations wrap a tuple of CopyableIndirections to Exprs. private: - using Container = - std::conditional_t>>, - std::tuple>...>>; + using Container = std::conditional_t>>, + std::tuple>...>>; public: CLASS_BOILERPLATE(Operation) @@ -437,14 +437,14 @@ public: private: parser::CharBlock name_; - CopyableIndirection> lower_, upper_, stride_; - CopyableIndirection> values_; + common::CopyableIndirection> lower_, upper_, stride_; + common::CopyableIndirection> values_; }; template struct ArrayConstructorValue { using Result = RESULT; EVALUATE_UNION_CLASS_BOILERPLATE(ArrayConstructorValue) - std::variant>, ImpliedDo> u; + std::variant>, ImpliedDo> u; }; template class ArrayConstructorValues { @@ -489,7 +489,7 @@ public: const Expr &LEN() const { return length_.value(); } private: - CopyableIndirection> length_; + common::CopyableIndirection> length_; }; template<> @@ -747,14 +747,19 @@ class Expr> : public ExpressionBase> { public: using Result = SomeKind; EVALUATE_UNION_CLASS_BOILERPLATE(Expr) - int GetKind() const { - return std::visit( - [](const auto &x) { return std::decay_t::Result::kind; }, - u); - } + int GetKind() const; common::MapTemplate> u; }; +template<> class Expr : public ExpressionBase { +public: + using Result = SomeCharacter; + EVALUATE_UNION_CLASS_BOILERPLATE(Expr) + int GetKind() const; + Expr LEN() const; + common::MapTemplate> u; +}; + // BOZ literal "typeless" constants must be wide enough to hold a numeric // value of any supported kind of INTEGER or REAL. They must also be // distinguishable from other integer constants, since they are permitted @@ -807,10 +812,10 @@ public: }; // This wrapper class is used, by means of a forward reference with -// OwningPointer, to implement owning pointers to analyzed expressions -// from parse tree nodes. +// an owning pointer, to cache analyzed expressions in parse tree nodes. struct GenericExprWrapper { GenericExprWrapper(Expr &&x) : v{std::move(x)} {} + ~GenericExprWrapper(); bool operator==(const GenericExprWrapper &) const; Expr v; }; diff --git a/flang/lib/evaluate/fold.cc b/flang/lib/evaluate/fold.cc index 9a41c9a92bd3..a72853b667fa 100644 --- a/flang/lib/evaluate/fold.cc +++ b/flang/lib/evaluate/fold.cc @@ -250,7 +250,7 @@ public: } private: - bool FoldArray(const CopyableIndirection> &expr) { + bool FoldArray(const common::CopyableIndirection> &expr) { Expr folded{Fold(context_, common::Clone(expr.value()))}; if (auto *c{UnwrapExpr>(folded)}) { // Copy elements in Fortran array element order diff --git a/flang/lib/evaluate/type.h b/flang/lib/evaluate/type.h index 0ea33443da69..1bdd3965e4e0 100644 --- a/flang/lib/evaluate/type.h +++ b/flang/lib/evaluate/type.h @@ -195,7 +195,7 @@ using SameKind = Type::kind>; // Many expressions, including subscripts, CHARACTER lengths, array bounds, // and effective type parameter values, are of a maximal kind of INTEGER. using IndirectSubscriptIntegerExpr = - CopyableIndirection>; + common::CopyableIndirection>; // A predicate that is true when a kind value is a kind that could possibly // be supported for an intrinsic type category on some target instruction diff --git a/flang/lib/evaluate/variable.cc b/flang/lib/evaluate/variable.cc index 05fae732c129..ecdbbd37b63d 100644 --- a/flang/lib/evaluate/variable.cc +++ b/flang/lib/evaluate/variable.cc @@ -258,8 +258,8 @@ std::ostream &Emit( return o; } -template -std::ostream &Emit(std::ostream &o, const CopyableIndirection &p, +template +std::ostream &Emit(std::ostream &o, const common::Indirection &p, const char *kw = nullptr) { if (kw != nullptr) { o << kw; diff --git a/flang/lib/evaluate/variable.h b/flang/lib/evaluate/variable.h index a572f3001798..28f3ac8f3ac1 100644 --- a/flang/lib/evaluate/variable.h +++ b/flang/lib/evaluate/variable.h @@ -77,7 +77,7 @@ public: CLASS_BOILERPLATE(Component) Component(const DataRef &b, const Symbol &c) : base_{b}, symbol_{&c} {} Component(DataRef &&b, const Symbol &c) : base_{std::move(b)}, symbol_{&c} {} - Component(CopyableIndirection &&b, const Symbol &c) + Component(common::CopyableIndirection &&b, const Symbol &c) : base_{std::move(b)}, symbol_{&c} {} const DataRef &base() const { return base_.value(); } @@ -90,7 +90,7 @@ public: std::ostream &AsFortran(std::ostream &) const; private: - CopyableIndirection base_; + common::CopyableIndirection base_; const Symbol *symbol_; }; @@ -238,7 +238,7 @@ public: private: std::vector base_; std::vector> subscript_, cosubscript_; - std::optional>> stat_, team_; + std::optional>> stat_, team_; bool teamIsTeamNumber_{false}; // false: TEAM=, true: TEAM_NUMBER= }; diff --git a/flang/lib/parser/parse-tree.cc b/flang/lib/parser/parse-tree.cc index b65525d22c5b..a4a6cb47204b 100644 --- a/flang/lib/parser/parse-tree.cc +++ b/flang/lib/parser/parse-tree.cc @@ -18,6 +18,13 @@ #include "../common/indirection.h" #include +// So "delete Expr::typedExpr;" calls an external destructor. +namespace Fortran::evaluate { +struct GenericExprWrapper { + ~GenericExprWrapper(); +}; +} + namespace Fortran::parser { // R867 @@ -171,3 +178,5 @@ std::ostream &operator<<(std::ostream &os, const CharBlock &x) { return os << x.ToString(); } } + +template class std::unique_ptr; diff --git a/flang/lib/parser/parse-tree.h b/flang/lib/parser/parse-tree.h index 1191d788db68..0d9bf5a6e52b 100644 --- a/flang/lib/parser/parse-tree.h +++ b/flang/lib/parser/parse-tree.h @@ -33,6 +33,7 @@ #include "../common/indirection.h" #include #include +#include #include #include #include @@ -66,14 +67,9 @@ class DerivedTypeSpec; // Expressions in the parse tree have owning pointers that can be set to // type-checked generic expression representations by semantic analysis. -// OwningPointer<> is used for leak safety without having to include -// the bulk of lib/evaluate/*.h headers into the parser proper. namespace Fortran::evaluate { struct GenericExprWrapper; // forward definition, wraps Expr } -namespace Fortran::common { -extern template class OwningPointer; -} // Most non-template classes in this file use these default definitions // for their move constructor and move assignment operator=, and disable @@ -553,7 +549,7 @@ WRAPPER_CLASS(NamedConstant, Name); // R1023 defined-binary-op -> . letter [letter]... . // R1414 local-defined-operator -> defined-unary-op | defined-binary-op // R1415 use-defined-operator -> defined-unary-op | defined-binary-op -// The Name here is stored with the dots; e.g., .FOO. +// The Name here is stored without the dots; e.g., FOO, not .FOO. WRAPPER_CLASS(DefinedOpName, Name); // R608 intrinsic-operator -> @@ -1695,8 +1691,10 @@ struct Expr { explicit Expr(Designator &&); explicit Expr(FunctionReference &&); - // Filled in after successful semantic analysis of the expression. - mutable common::OwningPointer typedExpr; + // Filled in with expression after successful semantic analysis. + mutable std::unique_ptr> + typedExpr; CharBlock source; @@ -2391,10 +2389,10 @@ struct ComputedGotoStmt { }; // R1162 stop-code -> scalar-default-char-expr | scalar-int-expr -// We can't distinguish character expressions from integer -// expressions during parsing, so we just parse an expr and -// check its type later. -WRAPPER_CLASS(StopCode, Scalar); +struct StopCode { + UNION_CLASS_BOILERPLATE(StopCode); + std::variant u; +}; // R1160 stop-stmt -> STOP [stop-code] [, QUIET = scalar-logical-expr] // R1161 error-stop-stmt -> @@ -2881,7 +2879,6 @@ struct GenericSpec { EMPTY_CLASS(ReadUnformatted); EMPTY_CLASS(WriteFormatted); EMPTY_CLASS(WriteUnformatted); - CharBlock source; std::variant u; diff --git a/flang/lib/semantics/assignment.h b/flang/lib/semantics/assignment.h index 8b7d2989be55..51981c68745c 100644 --- a/flang/lib/semantics/assignment.h +++ b/flang/lib/semantics/assignment.h @@ -17,6 +17,7 @@ #include "semantics.h" #include "../common/indirection.h" +#include "../evaluate/expression.h" namespace Fortran::parser { template struct Statement; @@ -31,12 +32,23 @@ struct ForallStmt; struct ForallConstruct; } +namespace Fortran::evaluate { +void CheckPointerAssignment(parser::ContextualMessages &, const Symbol &, + const evaluate::Expr &); +} + namespace Fortran::semantics { class AssignmentContext; +} +extern template class Fortran::common::Indirection< + Fortran::semantics::AssignmentContext>; + +namespace Fortran::semantics { class AssignmentChecker : public virtual BaseChecker { public: explicit AssignmentChecker(SemanticsContext &); + ~AssignmentChecker(); void Enter(const parser::AssignmentStmt &); void Enter(const parser::PointerAssignmentStmt &); void Enter(const parser::WhereStmt &); @@ -45,7 +57,7 @@ public: void Enter(const parser::ForallConstruct &); private: - common::ForwardReference context_; + common::Indirection context_; }; // Semantic analysis of an assignment statement or WHERE/FORALL construct. @@ -62,6 +74,5 @@ void AnalyzeAssignment( // well as in DO CONCURRENT loops. void AnalyzeConcurrentHeader( SemanticsContext &, const parser::ConcurrentHeader &); - } #endif // FORTRAN_SEMANTICS_ASSIGNMENT_H_ diff --git a/flang/lib/semantics/check-do-concurrent.cc b/flang/lib/semantics/check-do-concurrent.cc index 40ac612225c0..351de30c4e73 100644 --- a/flang/lib/semantics/check-do-concurrent.cc +++ b/flang/lib/semantics/check-do-concurrent.cc @@ -304,13 +304,13 @@ static CS GatherLocalVariableNames( static CS GatherReferencesFromExpression(const parser::Expr &expression) { // Use the new expression traversal framework if possible, for testing. - if (expression.typedExpr.has_value()) { + if (expression.typedExpr) { struct CollectSymbols : public virtual evaluate::VisitorBase { explicit CollectSymbols(int) {} void Handle(const Symbol *symbol) { result().push_back(symbol); } }; return evaluate::Visitor{0}.Traverse( - expression.typedExpr.value()); + *expression.typedExpr); } else { GatherSymbols gatherSymbols; parser::Walk(expression, gatherSymbols); @@ -361,7 +361,8 @@ public: auto &logicalExpr{ std::get(optionalLoopControl->u) .thing.thing}; - if (!ExpressionHasTypeCategory(logicalExpr.value().typedExpr.value(), + CHECK(logicalExpr.value().typedExpr); + if (!ExpressionHasTypeCategory(*logicalExpr.value().typedExpr, common::TypeCategory::Logical)) { messages_.Say(currentStatementSourcePosition_, "DO WHILE must have LOGICAL expression"_err_en_US); @@ -528,16 +529,19 @@ private: } // namespace Fortran::semantics -DEFINE_OWNING_DESTRUCTOR(ForwardReference, semantics::DoConcurrentContext) - namespace Fortran::semantics { DoConcurrentChecker::DoConcurrentChecker(SemanticsContext &context) : context_{new DoConcurrentContext{context}} {} +DoConcurrentChecker::~DoConcurrentChecker() = default; + // DO loops must be canonicalized prior to calling void DoConcurrentChecker::Leave(const parser::DoConstruct &x) { context_.value().Check(x); } } // namespace Fortran::semantics + +template class Fortran::common::Indirection< + Fortran::semantics::DoConcurrentContext>; diff --git a/flang/lib/semantics/check-do-concurrent.h b/flang/lib/semantics/check-do-concurrent.h index 8f20393ae197..dee1a4f4f595 100644 --- a/flang/lib/semantics/check-do-concurrent.h +++ b/flang/lib/semantics/check-do-concurrent.h @@ -21,17 +21,21 @@ namespace Fortran::parser { struct DoConstruct; } - namespace Fortran::semantics { class DoConcurrentContext; +} +extern template class Fortran::common::Indirection< + Fortran::semantics::DoConcurrentContext>; +namespace Fortran::semantics { class DoConcurrentChecker : public virtual BaseChecker { public: explicit DoConcurrentChecker(SemanticsContext &); + ~DoConcurrentChecker(); void Leave(const parser::DoConstruct &); private: - common::ForwardReference context_; + common::Indirection context_; }; } diff --git a/flang/tools/f18/dump.cc b/flang/tools/f18/dump.cc index 19684161d604..25e75887e156 100644 --- a/flang/tools/f18/dump.cc +++ b/flang/tools/f18/dump.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved. +// Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -16,7 +16,7 @@ // Each is based on operator<< for that type. There are overloadings for // reference and pointer, and for dumping to a provided ostream or cerr. -#ifdef DEBUG +#ifdef DEBUGF18 #include diff --git a/flang/tools/f18/stub-evaluate.cc b/flang/tools/f18/stub-evaluate.cc index a5643fabd2c5..0460415f19a3 100644 --- a/flang/tools/f18/stub-evaluate.cc +++ b/flang/tools/f18/stub-evaluate.cc @@ -14,14 +14,16 @@ // The parse tree has slots in which pointers to typed expressions may be // placed. When using the parser without the expression library, as here, -// we need to stub out the dependence. +// we need to stub out the dependence on the external destructor, which +// will never actually be called. #include "../../lib/common/indirection.h" namespace Fortran::evaluate { struct GenericExprWrapper { - bool operator==(const GenericExprWrapper &) const { return false; } + ~GenericExprWrapper(); }; +GenericExprWrapper::~GenericExprWrapper() = default; } -DEFINE_OWNING_DESTRUCTOR(OwningPointer, evaluate::GenericExprWrapper) +DEFINE_DELETER(Fortran::evaluate::GenericExprWrapper)