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

[Tim Shen via llvm-commits]

timshen added inline comments.

================
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);
+}
----------------
dblaikie wrote:
> This test seems a bit complicated - lambdas producing lambdas, a std::vector, etc.
> 
> What are the actual behaviors you're trying to test here?
Sorry, if std::vector raises surprises, it's a mistake. Changed to SmallVector.

I'd like to test the order of the scope_exit runs. I label each scope_exit with a number (x for make_scope_exit(f(x))), and when the scope exits, the label is pushed to the vector "v". Then I inspect v to make sure of the order of labels.

Do you have any suggestions to make it simpler?

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


https://reviews.llvm.org/D22796