[flang] Remove OwningPointer, use unique_ptr better instead.
authorpeter klausler <pklausler@nvidia.com>
Wed, 20 Mar 2019 18:38:45 +0000 (11:38 -0700)
committerpeter klausler <pklausler@nvidia.com>
Wed, 20 Mar 2019 18:38:45 +0000 (11:38 -0700)
Original-commit: flang-compiler/f18@89aff868aaec0a9ab6c65e86458a3e23a93da7d1
Reviewed-on: https://github.com/flang-compiler/f18/pull/346
Tree-same-pre-rewrite: false

25 files changed:
flang/CMakeLists.txt
flang/documentation/C++style.md
flang/lib/FIR/afforestation.cc
flang/lib/common/idioms.h
flang/lib/common/indirection.h
flang/lib/common/unwrap.h
flang/lib/evaluate/call.h
flang/lib/evaluate/characteristics.cc
flang/lib/evaluate/characteristics.h
flang/lib/evaluate/common.h
flang/lib/evaluate/constant.h
flang/lib/evaluate/descender.h
flang/lib/evaluate/expression.cc
flang/lib/evaluate/expression.h
flang/lib/evaluate/fold.cc
flang/lib/evaluate/type.h
flang/lib/evaluate/variable.cc
flang/lib/evaluate/variable.h
flang/lib/parser/parse-tree.cc
flang/lib/parser/parse-tree.h
flang/lib/semantics/assignment.h
flang/lib/semantics/check-do-concurrent.cc
flang/lib/semantics/check-do-concurrent.h
flang/tools/f18/dump.cc
flang/tools/f18/stub-evaluate.cc

index 4c0f50e..b51177d 100644 (file)
@@ -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
index ce846a7..7c5b7a2 100644 (file)
@@ -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
index 3be8f7e..242a16b 100644 (file)
@@ -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<parser::Expr> &a) {
-  return &a.value().typedExpr.value();
+  return a.value().typedExpr.get();
 }
 
 struct LinearOp;
@@ -1296,9 +1296,9 @@ public:
       const std::vector<LinearLabelRef> &refs) {
     auto &cases{
         std::get<std::list<parser::CaseConstruct::Case>>(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<SwitchCaseStmt>(
         result.defLab, result.values, result.labels);
     return result;
index 524c94f..747125e 100644 (file)
@@ -135,7 +135,6 @@ template<typename A> struct ListItemCount {
   }
 
 // Given a const reference to a value, return a copy of the value.
-
 template<typename A> A Clone(const A &x) { return x; }
 }
 #endif  // FORTRAN_COMMON_IDIOMS_H_
index c35f10f..5ddc2ef 100644 (file)
 #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<FORWARD_TYPE>;
+// outside any namespace in a header before use, and
+//    template class Fortran::common::Indirection<FORWARD_TYPE>;
+// in one C++ source file later where a definition of the type is visible.
 
 #include "../common/idioms.h"
 #include <memory>
@@ -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<typename... ARGS> static Indirection Make(ARGS &&... args) {
     return {new A(std::forward<ARGS>(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<typename... ARGS> static Indirection Make(ARGS &&... args) {
     return {new A(std::forward<ARGS>(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<typename A> class OwningPointer {
-public:
-  using element_type = A;
-
-  OwningPointer() {}
-  OwningPointer(OwningPointer &&that) : p_{that.p_} { that.p_ = nullptr; }
-  explicit OwningPointer(std::unique_ptr<A> &&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_;
-  }
+template<typename A> using CopyableIndirection = Indirection<A, true>;
 
-  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<typename A> class ForwardReference {
+// 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<CopyableIndirection<>> works.
+template<typename A> class Deleter {
 public:
-  using element_type = A;
-
-  explicit ForwardReference(std::unique_ptr<A> &&that) : p_{that.release()} {}
-  explicit ForwardReference(A *&&p) : p_{p} {
-    CHECK(p_ && "assigning null pointer to ForwardReference");
-    p = nullptr;
-  }
-  ForwardReference(ForwardReference<A> &&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<A>; \
-  template<> CLASS<A>::~CLASS() { \
-    delete p_; \
-    p_ = nullptr; \
-  } \
-  }
-
-// Optional definitions for OwningPointer and ForwardReference
-#define DEFINE_OWNING_COPY_CONSTRUCTORS(CLASS, A) \
-  namespace Fortran::common { \
-  template<> CLASS<A>::CLASS(const A &that) : p_{new A(that)} {} \
-  template<> \
-  CLASS<A>::CLASS(const CLASS<A> &that) \
-    : p_{that.p_ ? new A(*that.p_) : nullptr} {} \
+#define DEFINE_DELETER(A) \
+  template<> void Fortran::common::Deleter<A>::operator()(A *p) const { \
+    delete p; \
   }
-#define DEFINE_OWNING_COPY_ASSIGNMENTS(CLASS, A) \
-  namespace Fortran::common { \
-  template<> CLASS<A> &CLASS<A>::operator=(const A &that) { \
-    delete p_; \
-    p_ = new A(that); \
-    return *this; \
-  } \
-  template<> CLASS<A> &CLASS<A>::operator=(const CLASS<A> &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_
index 4ac8591..d94be84 100644 (file)
@@ -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<typename A, typename... Bs>
 auto Unwrap(const std::variant<Bs...> &) -> std::add_const_t<A> *;
 template<typename A, typename B, bool COPY>
 auto Unwrap(const Indirection<B, COPY> &) -> Constify<A, B> *;
-template<typename A, typename B>
-auto Unwrap(const OwningPointer<B> &) -> Constify<A, B> *;
-template<typename A, typename B>
-auto Unwrap(const CountedReference<B> &) -> Constify<A, B> *;
 
 // Implementations of specializations
 template<typename A, typename B> auto Unwrap(B *p) -> Constify<A, B> * {
@@ -145,15 +141,6 @@ auto Unwrap(const Indirection<B, COPY> &p) -> Constify<A, B> * {
 }
 
 template<typename A, typename B>
-auto Unwrap(const OwningPointer<B> &p) -> Constify<A, B> * {
-  if (p.get() != nullptr) {
-    return Unwrap<A>(*p);
-  } else {
-    return nullptr;
-  }
-}
-
-template<typename A, typename B>
 auto Unwrap(const CountedReference<B> &p) -> Constify<A, B> * {
   if (p.get() != nullptr) {
     return Unwrap<A>(*p);
index b0b39f8..0ae8565 100644 (file)
@@ -34,7 +34,7 @@ namespace Fortran::evaluate {
 class ActualArgument {
 public:
   explicit ActualArgument(Expr<SomeType> &&x) : value_{std::move(x)} {}
-  explicit ActualArgument(CopyableIndirection<Expr<SomeType>> &&v)
+  explicit ActualArgument(common::CopyableIndirection<Expr<SomeType>> &&v)
     : value_{std::move(v)} {}
 
   Expr<SomeType> &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<Expr<SomeType>> value_;
+  common::CopyableIndirection<Expr<SomeType>> value_;
 };
 
 using ActualArguments = std::vector<std::optional<ActualArgument>>;
index 79dad84..284012a 100644 (file)
@@ -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)
index d1389e1..14188a5 100644 (file)
@@ -54,7 +54,7 @@ struct DummyDataObject {
 struct DummyProcedure {
   ENUM_CLASS(Attr, Pointer, Optional)
   DEFAULT_CONSTRUCTORS_AND_ASSIGNMENTS(DummyProcedure)
-  common::OwningPointer<Procedure> explicitProcedure;
+  std::unique_ptr<Procedure, common::Deleter<Procedure>> explicitProcedure;
   common::EnumSet<Attr, 32> attrs;
   bool operator==(const DummyProcedure &) const;
   std::ostream &Dump(std::ostream &) const;
index 3587869..71ca160 100644 (file)
@@ -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<typename A> using CopyableIndirection = common::Indirection<A, true>;
-
 // Forward definition of Expr<> so that it can be indirectly used in its own
 // definition
 template<typename A> class Expr;
index aa81ea1..cb33967 100644 (file)
@@ -128,8 +128,8 @@ private:
   std::vector<std::int64_t> shape_;
 };
 
-using StructureConstructorValues =
-    std::map<const semantics::Symbol *, CopyableIndirection<Expr<SomeType>>>;
+using StructureConstructorValues = std::map<const semantics::Symbol *,
+    common::CopyableIndirection<Expr<SomeType>>>;
 
 template<>
 class Constant<SomeDerived>
index 4dea229..d4d8c25 100644 (file)
@@ -51,10 +51,12 @@ public:
     }
   }
 
-  template<typename X> void Descend(const CopyableIndirection<X> &p) {
+  template<typename X, bool COPY>
+  void Descend(const common::Indirection<X, COPY> &p) {
     Visit(p.value());
   }
-  template<typename X> void Descend(CopyableIndirection<X> &p) {
+  template<typename X, bool COPY>
+  void Descend(common::Indirection<X, COPY> &p) {
     Visit(p.value());
   }
 
index 0f4f068..d286a70 100644 (file)
@@ -86,7 +86,8 @@ std::ostream &LogicalOperation<KIND>::Infix(std::ostream &o) const {
 }
 
 template<typename T>
-std::ostream &Emit(std::ostream &o, const CopyableIndirection<Expr<T>> &expr) {
+std::ostream &Emit(
+    std::ostream &o, const common::CopyableIndirection<Expr<T>> &expr) {
   return expr.value().AsFortran(o);
 }
 
@@ -147,7 +148,7 @@ std::ostream &ExpressionBase<RESULT>::AsFortran(std::ostream &o) const {
             o << "z'" << x.Hexadecimal() << "'";
           },
           [&](const NullPointer &) { o << "NULL()"; },
-          [&](const CopyableIndirection<Substring> &s) {
+          [&](const common::CopyableIndirection<Substring> &s) {
             s.value().AsFortran(o);
           },
           [&](const ImpliedDoIndex &i) { o << i.name.ToString(); },
@@ -184,7 +185,7 @@ Expr<SubscriptInteger> Expr<Type<TypeCategory::Character, KIND>>::LEN() const {
       u);
 }
 
-Expr<SomeType>::~Expr() {}
+Expr<SomeType>::~Expr() = default;
 
 #if defined(__APPLE__) && defined(__GNUC__)
 template<typename A>
@@ -312,10 +313,28 @@ std::ostream &DerivedTypeSpecAsFortran(
   return o;
 }
 
+GenericExprWrapper::~GenericExprWrapper() = default;
+
 bool GenericExprWrapper::operator==(const GenericExprWrapper &that) const {
   return v == that.v;
 }
 
+template<TypeCategory CAT> int Expr<SomeKind<CAT>>::GetKind() const {
+  return std::visit(
+      [](const auto &kx) { return std::decay_t<decltype(kx)>::Result::kind; },
+      u);
+}
+
+int Expr<SomeCharacter>::GetKind() const {
+  return std::visit(
+      [](const auto &kx) { return std::decay_t<decltype(kx)>::Result::kind; },
+      u);
+}
+
+Expr<SubscriptInteger> Expr<SomeCharacter>::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)
index b904e72..002e04d 100644 (file)
@@ -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<operands == 1, CopyableIndirection<Expr<Operand<0>>>,
-          std::tuple<CopyableIndirection<Expr<OPERANDS>>...>>;
+  using Container = std::conditional_t<operands == 1,
+      common::CopyableIndirection<Expr<Operand<0>>>,
+      std::tuple<common::CopyableIndirection<Expr<OPERANDS>>...>>;
 
 public:
   CLASS_BOILERPLATE(Operation)
@@ -437,14 +437,14 @@ public:
 
 private:
   parser::CharBlock name_;
-  CopyableIndirection<Expr<Index>> lower_, upper_, stride_;
-  CopyableIndirection<ArrayConstructorValues<Result>> values_;
+  common::CopyableIndirection<Expr<Index>> lower_, upper_, stride_;
+  common::CopyableIndirection<ArrayConstructorValues<Result>> values_;
 };
 
 template<typename RESULT> struct ArrayConstructorValue {
   using Result = RESULT;
   EVALUATE_UNION_CLASS_BOILERPLATE(ArrayConstructorValue)
-  std::variant<CopyableIndirection<Expr<Result>>, ImpliedDo<Result>> u;
+  std::variant<common::CopyableIndirection<Expr<Result>>, ImpliedDo<Result>> u;
 };
 
 template<typename RESULT> class ArrayConstructorValues {
@@ -489,7 +489,7 @@ public:
   const Expr<SubscriptInteger> &LEN() const { return length_.value(); }
 
 private:
-  CopyableIndirection<Expr<SubscriptInteger>> length_;
+  common::CopyableIndirection<Expr<SubscriptInteger>> length_;
 };
 
 template<>
@@ -747,14 +747,19 @@ class Expr<SomeKind<CAT>> : public ExpressionBase<SomeKind<CAT>> {
 public:
   using Result = SomeKind<CAT>;
   EVALUATE_UNION_CLASS_BOILERPLATE(Expr)
-  int GetKind() const {
-    return std::visit(
-        [](const auto &x) { return std::decay_t<decltype(x)>::Result::kind; },
-        u);
-  }
+  int GetKind() const;
   common::MapTemplate<Expr, CategoryTypes<CAT>> u;
 };
 
+template<> class Expr<SomeCharacter> : public ExpressionBase<SomeCharacter> {
+public:
+  using Result = SomeCharacter;
+  EVALUATE_UNION_CLASS_BOILERPLATE(Expr)
+  int GetKind() const;
+  Expr<SubscriptInteger> LEN() const;
+  common::MapTemplate<Expr, CategoryTypes<TypeCategory::Character>> 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<SomeType> &&x) : v{std::move(x)} {}
+  ~GenericExprWrapper();
   bool operator==(const GenericExprWrapper &) const;
   Expr<SomeType> v;
 };
index 9a41c9a..a72853b 100644 (file)
@@ -250,7 +250,7 @@ public:
   }
 
 private:
-  bool FoldArray(const CopyableIndirection<Expr<T>> &expr) {
+  bool FoldArray(const common::CopyableIndirection<Expr<T>> &expr) {
     Expr<T> folded{Fold(context_, common::Clone(expr.value()))};
     if (auto *c{UnwrapExpr<Constant<T>>(folded)}) {
       // Copy elements in Fortran array element order
index 0ea3344..1bdd396 100644 (file)
@@ -195,7 +195,7 @@ using SameKind = Type<CATEGORY, std::decay_t<T>::kind>;
 // Many expressions, including subscripts, CHARACTER lengths, array bounds,
 // and effective type parameter values, are of a maximal kind of INTEGER.
 using IndirectSubscriptIntegerExpr =
-    CopyableIndirection<Expr<SubscriptInteger>>;
+    common::CopyableIndirection<Expr<SubscriptInteger>>;
 
 // 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
index 05fae73..ecdbbd3 100644 (file)
@@ -258,8 +258,8 @@ std::ostream &Emit(
   return o;
 }
 
-template<typename A>
-std::ostream &Emit(std::ostream &o, const CopyableIndirection<A> &p,
+template<typename A, bool COPY>
+std::ostream &Emit(std::ostream &o, const common::Indirection<A, COPY> &p,
     const char *kw = nullptr) {
   if (kw != nullptr) {
     o << kw;
index a572f30..28f3ac8 100644 (file)
@@ -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<DataRef> &&b, const Symbol &c)
+  Component(common::CopyableIndirection<DataRef> &&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<DataRef> base_;
+  common::CopyableIndirection<DataRef> base_;
   const Symbol *symbol_;
 };
 
@@ -238,7 +238,7 @@ public:
 private:
   std::vector<const Symbol *> base_;
   std::vector<Expr<SubscriptInteger>> subscript_, cosubscript_;
-  std::optional<CopyableIndirection<Expr<SomeInteger>>> stat_, team_;
+  std::optional<common::CopyableIndirection<Expr<SomeInteger>>> stat_, team_;
   bool teamIsTeamNumber_{false};  // false: TEAM=, true: TEAM_NUMBER=
 };
 
index b65525d..a4a6cb4 100644 (file)
 #include "../common/indirection.h"
 #include <algorithm>
 
+// 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<Fortran::evaluate::GenericExprWrapper>;
index 1191d78..0d9bf5a 100644 (file)
@@ -33,6 +33,7 @@
 #include "../common/indirection.h"
 #include <cinttypes>
 #include <list>
+#include <memory>
 #include <optional>
 #include <string>
 #include <tuple>
@@ -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<SomeType>
 }
-namespace Fortran::common {
-extern template class OwningPointer<evaluate::GenericExprWrapper>;
-}
 
 // 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<evaluate::GenericExprWrapper> typedExpr;
+  // Filled in with expression after successful semantic analysis.
+  mutable std::unique_ptr<evaluate::GenericExprWrapper,
+      common::Deleter<evaluate::GenericExprWrapper>>
+      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<Expr>);
+struct StopCode {
+  UNION_CLASS_BOILERPLATE(StopCode);
+  std::variant<ScalarDefaultCharExpr, ScalarIntExpr> 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<Name, DefinedOperator, Assignment, ReadFormatted,
       ReadUnformatted, WriteFormatted, WriteUnformatted>
       u;
index 8b7d298..51981c6 100644 (file)
@@ -17,6 +17,7 @@
 
 #include "semantics.h"
 #include "../common/indirection.h"
+#include "../evaluate/expression.h"
 
 namespace Fortran::parser {
 template<typename> struct Statement;
@@ -31,12 +32,23 @@ struct ForallStmt;
 struct ForallConstruct;
 }
 
+namespace Fortran::evaluate {
+void CheckPointerAssignment(parser::ContextualMessages &, const Symbol &,
+    const evaluate::Expr<evaluate::SomeType> &);
+}
+
 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<AssignmentContext> context_;
+  common::Indirection<AssignmentContext> 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_
index 40ac612..351de30 100644 (file)
@@ -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<CS> {
       explicit CollectSymbols(int) {}
       void Handle(const Symbol *symbol) { result().push_back(symbol); }
     };
     return evaluate::Visitor<CS, CollectSymbols>{0}.Traverse(
-        expression.typedExpr.value());
+        *expression.typedExpr);
   } else {
     GatherSymbols gatherSymbols;
     parser::Walk(expression, gatherSymbols);
@@ -361,7 +361,8 @@ public:
         auto &logicalExpr{
             std::get<parser::ScalarLogicalExpr>(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>;
index 8f20393..dee1a4f 100644 (file)
 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<DoConcurrentContext> context_;
+  common::Indirection<DoConcurrentContext> context_;
 };
 
 }
index 1968416..25e7588 100644 (file)
@@ -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 <iostream>
 
index a5643fa..0460415 100644 (file)
 
 // 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)