[flang-commits] [flang] d590c85 - [flang] Fix pimpl idiom for IntrinsicProcTable.

[Michael Kruse via flang-commits]

Author: Michael Kruse
Date: 2020-10-24T00:28:05-05:00
New Revision: d590c854303959bcaa78746a62b9a8f81583c111

URL: https://github.com/llvm/llvm-project/commit/d590c854303959bcaa78746a62b9a8f81583c111
DIFF: https://github.com/llvm/llvm-project/commit/d590c854303959bcaa78746a62b9a8f81583c111.diff

LOG: [flang] Fix pimpl idiom for IntrinsicProcTable.

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

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/flang/include/flang/Common/idioms.h b/flang/include/flang/Common/idioms.h
index 6e499eb04f36..84a8fd5be4cb 100644
--- a/flang/include/flang/Common/idioms.h
+++ b/flang/include/flang/Common/idioms.h
@@ -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; }
 

diff  --git a/flang/include/flang/Evaluate/intrinsics.h b/flang/include/flang/Evaluate/intrinsics.h
index 2cd9a3505588..14aff0db0175 100644
--- a/flang/include/flang/Evaluate/intrinsics.h
+++ b/flang/include/flang/Evaluate/intrinsics.h
@@ -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 @@ class IntrinsicProcTable {
   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

diff  --git a/flang/lib/Evaluate/intrinsics.cpp b/flang/lib/Evaluate/intrinsics.cpp
index eaff9620a22d..7c00ac6d8195 100644
--- a/flang/lib/Evaluate/intrinsics.cpp
+++ b/flang/lib/Evaluate/intrinsics.cpp
@@ -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;
 }