[PATCH] D22796: [ADT] Add make_scope_exit().

[David Blaikie via llvm-commits]

dblaikie added a subscriber: dblaikie.

================
Comment at: include/llvm/ADT/ScopeExit.h:29
@@ +28,3 @@
+public:
+  scope_exit() = delete;
+
----------------
This is probably unnecessary - no default ctor is provided if you have any other dtor, right?

================
Comment at: include/llvm/ADT/ScopeExit.h:45
@@ +44,3 @@
+template <typename Callable>
+LLVM_ATTRIBUTE_UNUSED_RESULT
+detail::scope_exit<typename std::decay<Callable>::type>
----------------
You could consider putting the attribute on the scope_exit type? ('spose it doesn't make much difference)



================
Comment at: unittests/ADT/ScopeExitTest.cpp:21-31
@@ +20,13 @@
+TEST(ScopeExitTest, Copy) {
+  std::vector<int> v;
+  const auto f = [&](int counter) {
+    return [&v, counter] { v.push_back(counter); };
+  };
+  {
+    auto g1 = make_scope_exit(f(0));
+    { auto g2 = make_scope_exit(f(1)); }
+    auto g3 = make_scope_exit(f(2));
+    EXPECT_EQ((std::vector<int>{1}), v);
+  }
+  EXPECT_EQ((std::vector<int>{1, 2, 0}), v);
+}
----------------
This test seems a bit complicated - lambdas producing lambdas, a std::vector, etc.

What are the actual behaviors you're trying to test here?

================
Comment at: unittests/ADT/ScopeExitTest.cpp:37
@@ +36,3 @@
+  std::function<void()> f([&succ] { succ = true; });
+  ASSERT_TRUE(static_cast<bool>(f));
+  {
----------------
I take it this static cast is needed for some reason? (but isn't needed for EXPECT_FALSE?)


https://reviews.llvm.org/D22796