Moved object destructor should not log
authorAravind Anbudurai <aru7@fb.com>
Sat, 2 Jul 2016 02:27:31 +0000 (19:27 -0700)
committerFacebook Github Bot 8 <facebook-github-bot-8-bot@fb.com>
Sat, 2 Jul 2016 02:38:31 +0000 (19:38 -0700)
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

folly/experimental/AutoTimer.h
folly/experimental/test/AutoTimerTest.cpp

index d51be4a211a2890aca9df0c2bf117b7183fb6652..580be380c1b7ce4935056f0321ffa83eabfd37d9 100644 (file)
@@ -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_;
index 791092eae364f7bfcb7584dbaac1819e0b6af2c4..de8e3134443e55efefa4ae3368eb2efcabce100b 100644 (file)
@@ -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");
+}