From 5c921a21c409106e297b314289b18497bf32f1f1 Mon Sep 17 00:00:00 2001
From: florijan <florijan@memgraph.io>
Date: Fri, 11 Aug 2017 09:32:46 +0200
Subject: [PATCH] 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 | 5 ++-
src/utils/auto_scope.hpp | 63 ------------------------------
src/utils/on_scope_exit.hpp | 34 ++++++++++++++++
src/utils/stacktrace.hpp | 4 +-
src/utils/terminate_handler.hpp | 1 -
tests/unit/utils_on_scope_exit.cpp | 15 +++++++
6 files changed, 54 insertions(+), 68 deletions(-)
delete mode 100644 src/utils/auto_scope.hpp
create mode 100644 src/utils/on_scope_exit.hpp
create mode 100644 tests/unit/utils_on_scope_exit.cpp
diff --git a/poc/memory/block_allocator.hpp b/poc/memory/block_allocator.hpp
index cdb7c41d1..7eb7452d5 100644
--- a/poc/memory/block_allocator.hpp
+++ b/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;
}
diff --git a/src/utils/auto_scope.hpp b/src/utils/auto_scope.hpp
deleted file mode 100644
index 7fdecc8be..000000000
--- a/src/utils/auto_scope.hpp
+++ /dev/null
@@ -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
diff --git a/src/utils/on_scope_exit.hpp b/src/utils/on_scope_exit.hpp
new file mode 100644
index 000000000..9b63e7f86
--- /dev/null
+++ b/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_;
+};
+}
diff --git a/src/utils/stacktrace.hpp b/src/utils/stacktrace.hpp
index a29e85af6..20556e402 100644
--- a/src/utils/stacktrace.hpp
+++ b/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);
diff --git a/src/utils/terminate_handler.hpp b/src/utils/terminate_handler.hpp
index 9b6a7ceef..e818e61dd 100644
--- a/src/utils/terminate_handler.hpp
+++ b/src/utils/terminate_handler.hpp
@@ -1,6 +1,5 @@
#pragma once
-#include "utils/auto_scope.hpp"
#include "utils/stacktrace.hpp"
#include <execinfo.h>
diff --git a/tests/unit/utils_on_scope_exit.cpp b/tests/unit/utils_on_scope_exit.cpp
new file mode 100644
index 000000000..b41ef85d6
--- /dev/null
+++ b/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);
+}