[flang] Fix pimpl idiom for IntrinsicProcTable.
authorMichael Kruse <llvm-project@meinersbur.de>
Sat, 24 Oct 2020 03:59:57 +0000 (22:59 -0500)
committerMichael Kruse <llvm-project@meinersbur.de>
Sat, 24 Oct 2020 05:28:05 +0000 (00:28 -0500)
The class IntrinsicProcTable uses the pimpl idiom and manages its own pointer-to-implementation.  However, it violates the rule-of-five and does not implement a move-constructor or assignment-operator. Due to differences between compilers in implementation copy elision, these may or may not be used. Due to the missing user implementation for resource handling, using the results in runtime errors.

Fix my using `std::unique_ptr` instead of custom resource management.

Reviewed By: klausler

Differential Revision: https://reviews.llvm.org/D88794

flang/include/flang/Common/idioms.h
flang/include/flang/Evaluate/intrinsics.h
flang/lib/Evaluate/intrinsics.cpp

index 6e499eb..84a8fd5 100644 (file)
@@ -26,6 +26,7 @@
 #include "llvm/Support/Compiler.h"
 #include <functional>
 #include <list>
+#include <memory>
 #include <optional>
 #include <string>
 #include <tuple>
@@ -142,6 +143,14 @@ template <typename T> constexpr T &Deref(T *p, const char *file, int line) {
   return *p;
 }
 
+template <typename T>
+constexpr T &Deref(const std::unique_ptr<T> &p, const char *file, int line) {
+  if (!p) {
+    Fortran::common::die("nullptr dereference at %s(%d)", file, line);
+  }
+  return *p;
+}
+
 // Given a const reference to a value, return a copy of the value.
 template <typename A> A Clone(const A &x) { return x; }
 
index 2cd9a35..14aff0d 100644 (file)
@@ -15,6 +15,7 @@
 #include "flang/Common/default-kinds.h"
 #include "flang/Parser/char-block.h"
 #include "flang/Parser/message.h"
+#include <memory>
 #include <optional>
 #include <string>
 
@@ -64,8 +65,12 @@ class IntrinsicProcTable {
 private:
   class Implementation;
 
+  IntrinsicProcTable() = default;
+
 public:
   ~IntrinsicProcTable();
+  IntrinsicProcTable(IntrinsicProcTable &&) = default;
+
   static IntrinsicProcTable Configure(
       const common::IntrinsicTypeDefaultKinds &);
 
@@ -100,7 +105,7 @@ public:
   llvm::raw_ostream &Dump(llvm::raw_ostream &) const;
 
 private:
-  Implementation *impl_{nullptr}; // owning pointer
+  std::unique_ptr<Implementation> impl_;
 };
 
 // Check if an intrinsic explicitly allows its INTENT(OUT) arguments to be
index eaff962..7c00ac6 100644 (file)
@@ -2221,16 +2221,12 @@ DynamicType IntrinsicProcTable::Implementation::GetSpecificType(
   return DynamicType{category, defaults_.GetDefaultKind(category)};
 }
 
-IntrinsicProcTable::~IntrinsicProcTable() {
-  // Discard the configured tables.
-  delete impl_;
-  impl_ = nullptr;
-}
+IntrinsicProcTable::~IntrinsicProcTable() = default;
 
 IntrinsicProcTable IntrinsicProcTable::Configure(
     const common::IntrinsicTypeDefaultKinds &defaults) {
   IntrinsicProcTable result;
-  result.impl_ = new IntrinsicProcTable::Implementation(defaults);
+  result.impl_ = std::make_unique<IntrinsicProcTable::Implementation>(defaults);
   return result;
 }