[flang-commits] [PATCH] D88794: [flang] Fix pimpl idiom for IntrinsicProcTable.

[Michael Kruse via Phabricator via flang-commits]

Meinersbur created this revision.
Meinersbur added reviewers: isuruf, sscalpone, klausler, tskeith, DavidTruby.
Meinersbur added a project: Flang.
Herald added a reviewer: jdoerfert.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Meinersbur requested review of this revision.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88794

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


Index: flang/lib/Evaluate/intrinsics.cpp
===================================================================
--- flang/lib/Evaluate/intrinsics.cpp
+++ flang/lib/Evaluate/intrinsics.cpp
@@ -2070,16 +2070,12 @@
   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;
 }
 
Index: flang/include/flang/Evaluate/intrinsics.h
===================================================================
--- flang/include/flang/Evaluate/intrinsics.h
+++ 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 @@
 private:
   class Implementation;
 
+  IntrinsicProcTable() = default;
+
 public:
   ~IntrinsicProcTable();
+  IntrinsicProcTable(IntrinsicProcTable &&) = default;
+
   static IntrinsicProcTable Configure(
       const common::IntrinsicTypeDefaultKinds &);
 
@@ -100,7 +105,7 @@
   llvm::raw_ostream &Dump(llvm::raw_ostream &) const;
 
 private:
-  Implementation *impl_{nullptr}; // owning pointer
+  std::unique_ptr<Implementation> impl_;
 };
 } // namespace Fortran::evaluate
 #endif // FORTRAN_EVALUATE_INTRINSICS_H_
Index: flang/include/flang/Common/idioms.h
===================================================================
--- flang/include/flang/Common/idioms.h
+++ flang/include/flang/Common/idioms.h
@@ -142,6 +142,14 @@
   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; }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D88794.296029.patch
Type: text/x-patch
Size: 2285 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/flang-commits/attachments/20201004/987eec67/attachment-0001.bin>