From c7cfc3f357611062e3de6de70ee40ca813176746 Mon Sep 17 00:00:00 2001 From: peter klausler Date: Mon, 4 Mar 2019 15:15:08 -0800 Subject: [PATCH] [flang] Add ForwardReference<> + documentation + clean-up Original-commit: flang-compiler/f18@09e9501951499625fdabdc04f3bcf7ae85b37467 Reviewed-on: https://github.com/flang-compiler/f18/pull/314 --- flang/documentation/C++style.md | 5 +- flang/lib/common/indirection.h | 146 ++++++++++++++++++++++++---------- flang/lib/evaluate/characteristics.cc | 3 +- flang/lib/evaluate/expression.cc | 2 +- flang/lib/semantics/expression.cc | 3 +- 5 files changed, 110 insertions(+), 49 deletions(-) diff --git a/flang/documentation/C++style.md b/flang/documentation/C++style.md index 919e200..ce846a7 100644 --- a/flang/documentation/C++style.md +++ b/flang/documentation/C++style.md @@ -199,12 +199,13 @@ 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. -* `OwningReference<>`: A non-nullable `OwningPointer<>`. * `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. * `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 @@ -221,8 +222,8 @@ A feature matrix: | `unique_ptr<>` | yes | yes | yes | yes | no | no | | `shared_ptr<>` | yes | yes | yes | yes | shallowly | no | | `OwningPointer<>` | yes | yes | yes | yes | optionally deeply | yes | -| `OwningReference<>` | no | yes | yes | yes | optionally deeply | yes | | `Indirection<>` | no | n/a | yes | yes | optionally deeply | no | +| `ForwardReference<>` | no | n/a | yes | yes | optionally deeply | yes | | `CountedReference<>` | yes | yes | yes | yes | shallowly | no | ### Overall design preferences diff --git a/flang/lib/common/indirection.h b/flang/lib/common/indirection.h index 4cf822b..a04404f 100644 --- a/flang/lib/common/indirection.h +++ b/flang/lib/common/indirection.h @@ -15,11 +15,20 @@ #ifndef FORTRAN_COMMON_INDIRECTION_H_ #define FORTRAN_COMMON_INDIRECTION_H_ -// Defines a smart pointer class template that's rather like std::unique_ptr<> -// but further restricted, like a C++ reference, to be non-null when constructed -// or assigned. Users need not check whether these pointers are null. -// Supports copy construction, too. -// Intended to be as invisible as a reference, wherever possible. +// 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. #include "../common/idioms.h" #include @@ -61,6 +70,7 @@ public: A *operator->() { return p_; } const A *operator->() const { return p_; } + bool operator==(const A &x) const { return *p_ == x; } bool operator==(const Indirection &that) const { return *p_ == *that.p_; } template static Indirection Make(ARGS &&... args) { @@ -75,6 +85,7 @@ private: template class Indirection { public: using element_type = A; + Indirection() = delete; Indirection(A *&&p) : p_{p} { CHECK(p_ && "assigning null pointer to Indirection"); @@ -114,6 +125,7 @@ public: A *operator->() { return p_; } const A *operator->() const { return p_; } + bool operator==(const A &x) const { return *p_ == x; } bool operator==(const Indirection &that) const { return *p_ == *that.p_; } template static Indirection Make(ARGS &&... args) { @@ -133,15 +145,11 @@ public: using element_type = A; OwningPointer() {} - OwningPointer(OwningPointer &&that) : p_{that.release()} {} + 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; } - OwningPointer &operator=(OwningPointer &&that) { - reset(that.release()); - return *this; - } - // Must be externally defined; see the macro below. + // Must be externally defined; see DEFINE_OWNING_DESTRUCTOR below ~OwningPointer(); // Must be externally defined if copying is needed. @@ -150,6 +158,16 @@ public: 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)); + } + A &operator*() { return *p_; } const A &operator*() const { return *p_; } A *operator->() { return p_; } @@ -157,69 +175,111 @@ public: A *get() const { return p_; } - A *release() { - A *result{p_}; - p_ = nullptr; - return result; + 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_); } - void reset(A *p) { - this->~OwningPointer(); - p_ = 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; } - bool operator==(const A &x) const { - return p_ != nullptr && (p_ == &x || *p_ == x); + // 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; } - bool operator==(const OwningPointer &that) const { - return (p_ == nullptr && that.p_ == nullptr) || - (that.p_ != nullptr && *this == *that.p_); + ForwardReference &operator=(A *&&p) { + return *this = ForwardReference(std::move(p)); + } + + A &operator*() { return *p_; } + const A &operator*() const { return *p_; } + A *operator->() { return p_; } + const A *operator->() const { return 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}; }; +} // Mandatory instantiation and definition -- put somewhere, not in a namespace -#define DEFINE_OWNING_POINTER_DESTRUCTOR(A) \ +// CLASS here is OwningPointer or ForwardReference. +#define DEFINE_OWNING_DESTRUCTOR(CLASS, A) \ namespace Fortran::common { \ - template class OwningPointer; \ - template<> OwningPointer::~OwningPointer() { \ + template class CLASS; \ + template<> CLASS::~CLASS() { \ delete p_; \ p_ = nullptr; \ } \ } -// Optional definitions -#define DEFINE_OWNING_POINTER_COPY_CONSTRUCTORS(A) \ +// 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<> \ - OwningPointer::OwningPointer(const A &that) : p_{new A(that)} {} \ - template<> \ - OwningPointer::OwningPointer(const OwningPointer &that) \ + CLASS::CLASS(const CLASS &that) \ : p_{that.p_ ? new A(*that.p_) : nullptr} {} \ } -#define DEFINE_OWNING_POINTER_COPY_ASSIGNMENTS(A) \ +#define DEFINE_OWNING_COPY_ASSIGNMENTS(CLASS, A) \ namespace Fortran::common { \ - template<> OwningPointer &OwningPointer::operator=(const A &that) { \ + template<> CLASS &CLASS::operator=(const A &that) { \ delete p_; \ p_ = new A(that); \ return *this; \ } \ - template<> \ - OwningPointer &OwningPointer::operator=( \ - const OwningPointer &that) { \ + template<> CLASS &CLASS::operator=(const CLASS &that) { \ delete p_; \ p_ = that.p_ ? new A(*that.p_) : nullptr; \ return *this; \ } \ } -#define DEFINE_OWNING_POINTER_COPY_FUNCTIONS(A) \ - DEFINE_OWNING_POINTER_COPY_CONSTRUCTORS(A) \ - DEFINE_OWNING_POINTER_COPY_ASSIGNMENTS(A) -#define DEFINE_OWNING_POINTER_SPECIAL_FUNCTIONS(A) \ - DEFINE_OWNING_POINTER_DESTRUCTOR(A) \ - DEFINE_OWNING_POINTER_COPY_FUNCTIONS(A) -} +#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/evaluate/characteristics.cc b/flang/lib/evaluate/characteristics.cc index 088dd9c..b44f98e 100644 --- a/flang/lib/evaluate/characteristics.cc +++ b/flang/lib/evaluate/characteristics.cc @@ -101,4 +101,5 @@ std::ostream &Procedure::Dump(std::ostream &o) const { } // Define OwningPointer special member functions -DEFINE_OWNING_POINTER_SPECIAL_FUNCTIONS(evaluate::characteristics::Procedure) +DEFINE_OWNING_SPECIAL_FUNCTIONS( + OwningPointer, evaluate::characteristics::Procedure) diff --git a/flang/lib/evaluate/expression.cc b/flang/lib/evaluate/expression.cc index b48dc7f..5104ef9 100644 --- a/flang/lib/evaluate/expression.cc +++ b/flang/lib/evaluate/expression.cc @@ -321,4 +321,4 @@ FOR_EACH_INTRINSIC_KIND(template class ArrayConstructor) // 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_POINTER_SPECIAL_FUNCTIONS(evaluate::GenericExprWrapper) +DEFINE_OWNING_SPECIAL_FUNCTIONS(OwningPointer, evaluate::GenericExprWrapper) diff --git a/flang/lib/semantics/expression.cc b/flang/lib/semantics/expression.cc index 596a041..b6b4654 100644 --- a/flang/lib/semantics/expression.cc +++ b/flang/lib/semantics/expression.cc @@ -2032,8 +2032,7 @@ public: #if PMKDEBUG // checked->AsFortran(std::cout << "checked expression: ") << '\n'; #endif - expr.typedExpr.reset( - new evaluate::GenericExprWrapper{std::move(*checked)}); + expr.typedExpr = new evaluate::GenericExprWrapper{std::move(*checked)}; } else { #if PMKDEBUG std::cout << "TODO: expression analysis failed for this expression: "; -- 2.7.4