[flang-commits] [PATCH] D88797: [flang] Fix copy elision assumption.

[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.

Like D88794 <https://reviews.llvm.org/D88794>, the Restorer depends on copy elision to happen. Without copy elision, the function ScopedSet calls the move constructor before its dtor. The dtor will prematurely restore the reference to the original value.

Instead, it should work like `std::unique_ptr` where moving shifts the responsibility to the target. Introduce an `active_` member that stores whether the instance still holds the responsibility to restore.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88797

Files:
  flang/include/flang/Common/restorer.h


Index: flang/include/flang/Common/restorer.h
===================================================================
--- flang/include/flang/Common/restorer.h
+++ flang/include/flang/Common/restorer.h
@@ -22,10 +22,34 @@
 namespace Fortran::common {
 template <typename A> class Restorer {
 public:
-  explicit Restorer(A &p) : p_{p}, original_{std::move(p)} {}
-  ~Restorer() { p_ = std::move(original_); }
+  explicit Restorer(A &p) : active_{true}, p_{p}, original_{std::move(p)} {}
+  ~Restorer() { Release(); }
+
+  Restorer(const Restorer &) = delete;
+  Restorer(Restorer &&that)
+      : active_{that.active_}, p_{that.p_}, original_{
+                                                std::move(that.original_)} {
+    that.active_ = false;
+  }
+
+  const Restorer &operator=(const Restorer &) = delete;
+  const Restorer &operator=(Restorer &&that) {
+    Release();
+    if (that.active_) {
+      p_ = that.p_;
+      original_ = std::move(that.original_);
+      that.active = false;
+    }
+  }
 
 private:
+  void Release() {
+    if (active_)
+      p_ = std::move(original_);
+    active_ = false;
+  }
+
+  bool active_;
   A &p_;
   A original_;
 };


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D88797.296038.patch
Type: text/x-patch
Size: 1164 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/flang-commits/attachments/20201004/57814edd/attachment.bin>