From: Aravind Anbudurai <aru7@fb.com>
Date: Sat, 2 Jul 2016 02:27:31 +0000 (-0700)
Subject: Moved object destructor should not log
X-Git-Tag: 2016.07.26~88
X-Git-Url: http://plrg.eecs.uci.edu/git/?a=commitdiff_plain;h=3df144bef3e58428d62402fe0b8a3d812e59f9a5;p=folly.git

Moved object destructor should not log

Summary:
I introduced a helper method to make an AutoTimer and forced default move-ctor.
That caused moved object's destruction to log and that is undesirable.
This defines a custom move-ctor to set a direct the moved object to not log.

Reviewed By: yfeldblum

Differential Revision: D3511206

fbshipit-source-id: 38ae6de5fe76077c5e5ed10f64ebe959f5674fa7
---

diff --git a/folly/experimental/AutoTimer.h b/folly/experimental/AutoTimer.h
index d51be4a2..580be380 100644
--- a/folly/experimental/AutoTimer.h
+++ b/folly/experimental/AutoTimer.h
@@ -21,6 +21,7 @@
 
 #include <folly/Conv.h>
 #include <folly/Format.h>
+#include <folly/Optional.h>
 #include <folly/String.h>
 #include <glog/logging.h>
 
@@ -79,7 +80,9 @@ class AutoTimer final {
   AutoTimer& operator=(AutoTimer&&) = default;
 
   ~AutoTimer() {
-    log(destructionMessage_);
+    if (destructionMessage_) {
+      log(destructionMessage_.value());
+    }
   }
 
   DoubleSeconds log(StringPiece msg = "") {
@@ -110,7 +113,7 @@ class AutoTimer final {
     return duration;
   }
 
-  const std::string destructionMessage_;
+  Optional<std::string> destructionMessage_;
   std::chrono::time_point<Clock> start_ = Clock::now();
   DoubleSeconds minTimeToLog_;
   Logger logger_;
diff --git a/folly/experimental/test/AutoTimerTest.cpp b/folly/experimental/test/AutoTimerTest.cpp
index 791092ea..de8e3134 100644
--- a/folly/experimental/test/AutoTimerTest.cpp
+++ b/folly/experimental/test/AutoTimerTest.cpp
@@ -13,8 +13,8 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-#include <gtest/gtest.h>
 
+#include <gtest/gtest.h>
 #include <folly/experimental/AutoTimer.h>
 
 using namespace folly;
@@ -114,3 +114,27 @@ TEST(TestAutoTimer, HandleMinLogTime) {
   ASSERT_EQ(std::chrono::duration<double>(2), timer.log("foo"));
   ASSERT_EQ(std::chrono::duration<double>::zero().count(), StubLogger::t);
 }
+
+TEST(TestAutoTimer, MovedObjectDestructionDoesntLog) {
+  const std::vector<std::string> expectedMsgs = {
+      "BEFORE_MOVE", "AFTER_MOVE", "END"};
+  int32_t current = 0;
+  SCOPE_EXIT {
+    EXPECT_EQ(3, current);
+  };
+
+  auto timer = [&expectedMsgs, &current] {
+    auto oldTimer = folly::makeAutoTimer(
+        "END",
+        std::chrono::duration<double>::zero(),
+        [&expectedMsgs, &current](
+            StringPiece msg, const std::chrono::duration<double>&) {
+          EXPECT_EQ(expectedMsgs.at(current), msg);
+          current++;
+        });
+    oldTimer.log("BEFORE_MOVE");
+    auto newTimer = std::move(oldTimer); // force the move-ctor
+    return newTimer;
+  }();
+  timer.log("AFTER_MOVE");
+}