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

[David Blaikie via llvm-commits]

dblaikie accepted this revision.
This revision is now accepted and ready to land.

================
Comment at: unittests/ADT/ScopeExitTest.cpp:22-32
@@ +21,13 @@
+    EXPECT_FALSE(Called);
+  }
+  EXPECT_TRUE(Called);
+}
+
+TEST(ScopeExitTest, Move) {
+  struct Callable {
+    bool &CopyConstructed;
+    bool &MoveConstructed;
+
+    void operator()() const {}
+
+    Callable(bool &CopyConstructed, bool &MoveConstructed)
----------------
I'd probably change this test to have a move-only type to ensure that the the scope exit device doesn't accidentally depend on copyability anywhere, syntactically, even if it doesn't execute it dynamically.

In theory you could/should have both tests (one could imagine a bug where the thing works correctly for a move-only type, but when given a move-and-copy type, it accidentally copies), but that doesn't seem to be worth worrying about for my money as you sort of have to go out of your way to make such a problem & until the implementation is more complicated such that those sort of problems are a risk, I wouldn't bother testing it.

(& as a bonus, then there's not even any reason to have any tracking of which, if any, constructors, etc, were called - there is only move)

Honestly - at that point is the "Basic" test worth keeping? I'm not sure.

Picturing just one test, something like:

  struct Callable {
    bool &Called;
    Callable(bool &Called) : Called(Called) { }
    Callable(Callable &&RHS) : Called(RHS.Called) { }
    void operator()() { Called = true; }
  };
  bool Called = false;
  {
    auto g = make_scope_exit(Callable(Called));
    EXPECT_FALSE(Called);
  }
  EXPECT_TRUE(Called);


https://reviews.llvm.org/D22796