utils::auto_scope refactor

Summary: Changed on-scope-exit-mechanism from macro (with two auto-generated variables and an all-capturing lambda) to an explicitly created variable that takes an std::function argument.

Reviewers: buda, mislav.bradac, teon.banek

Reviewed By: buda

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D659

poc/memory/block_allocator.hpp

@@ -3,7 +3,7 @@
 #include <memory>
 #include <vector>
 
-#include "utils/auto_scope.hpp"
+#include "utils/on_scope_exit.hpp"
 
 /* @brief Allocates blocks of block_size and stores
  * the pointers on allocated blocks inside a vector.
@@ -41,7 +41,8 @@ class BlockAllocator {
     if (unused_.size() == 0) unused_.emplace_back();
 
     auto ptr = unused_.back().data;
-    Auto(unused_.pop_back());
+    // TODO is it necessary to use OnScopeExit here? ptr is copied by value, right?
+    utils::OnScopeExit on_exit([this]() { unused_.pop_back(); });
     return ptr;
   }
 

src/utils/auto_scope.hpp

@@ -1,63 +0,0 @@
-#pragma once
-
-#include <utility>
-
-/**
- * @brief Calls a cleanup function on scope exit
- *
- * consider this example:
- *
- * void hard_worker()
- * {
- *     resource.enable();
- *     do_stuff();          // throws exception
- *     resource.disable();
- * }
- *
- * if do_stuff throws an exception, resource.disable is never called
- * and the app is left in an inconsistent state. ideally, you would like
- * to call resource.disable regardles of the exception being thrown.
- * OnScopeExit makes this possible and very convenient via a 'Auto' macro
- *
- * void hard_worker()
- * {
- *     resource.enable();
- *     Auto(resource.disable());
- *     do_stuff();          // throws exception
- * }
- *
- * now, resource.disable will be called every time it goes out of scope
- * regardless of the exception
- *
- * @tparam F Lambda which holds a wrapper function around the cleanup code
- */
-template <class F>
-class OnScopeExit {
- public:
-  OnScopeExit(F& f) : f(f) {}
-
-  ~OnScopeExit() {
-    // calls the cleanup function
-    f();
-  }
-
- private:
-  F& f;
-};
-
-#define TOKEN_PASTEx(x, y) x##y
-#define TOKEN_PASTE(x, y) TOKEN_PASTEx(x, y)
-
-#define Auto_INTERNAL(Destructor, counter)                             \
-  auto TOKEN_PASTE(auto_func_, counter) = [&]() { Destructor; };       \
-  OnScopeExit<decltype(TOKEN_PASTE(auto_func_, counter))> TOKEN_PASTE( \
-      auto_, counter)(TOKEN_PASTE(auto_func_, counter));
-
-#define Auto(Destructor) Auto_INTERNAL(Destructor, __COUNTER__)
-
-// -- example:
-// Auto(f());
-// -- is expended to:
-// auto auto_func_1 = [&]() { f(); };
-// OnScopeExit<decltype(auto_func_1)> auto_1(auto_func_1);
-// -- f() is called at the end of a scope

src/utils/on_scope_exit.hpp

@@ -0,0 +1,34 @@
+#pragma once
+
+#include <functional>
+
+namespace utils {
+
+/**
+ * Calls a function in it's destructor (on scope exit).
+ *
+ * Example usage:
+ *
+ * void long_function() {
+ *     resource.enable();
+ *     // long block of code, might throw an exception
+ *     resource.disable(); // we want this to happen for sure, and function end
+ * }
+ *
+ * Can be nicer and safer:
+ *
+ * void long_function() {
+ *     resource.enable();
+ *     OnScopeExit on_exit([&resource] { resource.disable(); });
+ *     // long block of code, might trow an exception
+ * }
+ */
+class OnScopeExit {
+ public:
+  OnScopeExit(const std::function<void()> function) : function_(function) {}
+  ~OnScopeExit() { function_(); }
+
+ private:
+  std::function<void()> function_;
+};
+}

src/utils/stacktrace.hpp

@@ -5,7 +5,7 @@
 #include <fmt/format.h>
 #include <stdexcept>
 
-#include "utils/auto_scope.hpp"
+#include "utils/on_scope_exit.hpp"
 
 class Stacktrace {
  public:
@@ -28,7 +28,7 @@ class Stacktrace {
 
     // will this leak if backtrace_symbols throws?
     char **symbols = nullptr;
-    Auto(free(symbols));
+    utils::OnScopeExit on_exit([&symbols]() { free(symbols); });
 
     symbols = backtrace_symbols(addresses, depth);
 

src/utils/terminate_handler.hpp

@@ -1,6 +1,5 @@
 #pragma once
 
-#include "utils/auto_scope.hpp"
 #include "utils/stacktrace.hpp"
 
 #include <execinfo.h>

tests/unit/utils_on_scope_exit.cpp

@@ -0,0 +1,15 @@
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include "utils/on_scope_exit.hpp"
+
+
+TEST(OnScopeExit, BasicUsage) {
+  int variable = 1;
+  {
+    ASSERT_EQ(variable, 1);
+    utils::OnScopeExit on_exit([&variable] { variable = 2; });
+    EXPECT_EQ(variable, 1);
+  }
+  EXPECT_EQ(variable, 2);
+}