[ADT] Refactor enumerate unit tests

[scott.linder]

Summary

Preparation for landing the tests for llvm::makeVisitor, including
breaking out the a "Counted" base class and explicitly testing
the prvalue case as distinct from the rvalue case.

[dblaikie]

llvm/unittests/ADT/STLExtrasTest.cpp

157

Probably doesn't need to be virtual, if no one ever destroys it polymorphically? But I guess we have warnings enabled that make that problematic?

Could we use composition instead, then? (Range contains a Counted, rather than derives from?)

[scott.linder]

llvm/unittests/ADT/STLExtrasTest.cpp

157

Good point, I think I just reflexively made it virtual. I would prefer to use inheritence as it cuts down on the boilerplate in the derived classes to just using Counted::Counted.

It doesn't seem like we have any warnings enabled that complain here, so I just made the destructor non-virtual

[scott.linder]

Make Counted::~Counted non-virtual

[dblaikie]

Looks good, thanks!

llvm/unittests/ADT/STLExtrasTest.cpp

 template <> struct CanCopy<false> {
   CanCopy(const CanCopy &) = delete;
 
   CanCopy() = default;
   CanCopy(CanCopy &&) = default;
 };
 
 template <bool Moveable, bool Copyable>
-struct Range : CanMove<Moveable>, CanCopy<Copyable> {
-  explicit Range(int &C, int &M, int &D) : C(C), M(M), D(D) {}
-  Range(const Range &R) : CanCopy<Copyable>(R), C(R.C), M(R.M), D(R.D) { ++C; }
-  Range(Range &&R) : CanMove<Moveable>(std::move(R)), C(R.C), M(R.M), D(R.D) {
-    ++M;
-  }
-  ~Range() { ++D; }
-
+class Counted : CanMove<Moveable>, CanCopy<Copyable> {
   int &C;
   int &M;
   int &D;
 
+public:
+  explicit Counted(int &C, int &M, int &D) : C(C), M(M), D(D) {}
+  Counted(const Counted &O) : CanCopy<Copyable>(O), C(O.C), M(O.M), D(O.D) {
+    ++C;
+  }
+  Counted(Counted &&O)
+      : CanMove<Moveable>(std::move(O)), C(O.C), M(O.M), D(O.D) {
+    ++M;
+  }
+  ~Counted() { ++D; }
+};
+
+template <bool Moveable, bool Copyable>
+struct Range : Counted<Moveable, Copyable> {
+  using Counted<Moveable, Copyable>::Counted;
   int *begin() { return nullptr; }
   int *end() { return nullptr; }
 };
 
-TEST(STLExtrasTest, EnumerateLifetimeSemantics) {
-  // Test that when enumerating lvalues and rvalues, there are no surprise
-  // copies or moves.
+TEST(STLExtrasTest, EnumerateLifetimeSemanticsPRValue) {
+  int Copies = 0;
+  int Moves = 0;
+  int Destructors = 0;
+  {
+    auto E = enumerate(Range<true, false>(Copies, Moves, Destructors));
+    (void)E;
+    // Doesn't compile.  rvalue ranges must be moveable.
+    // auto E2 = enumerate(Range<false, true>(Copies, Moves, Destructors));
+    EXPECT_EQ(0, Copies);
+    EXPECT_EQ(1, Moves);
+    EXPECT_EQ(1, Destructors);
+  }
+  EXPECT_EQ(0, Copies);
+  EXPECT_EQ(1, Moves);
+  EXPECT_EQ(2, Destructors);
+}
 
+TEST(STLExtrasTest, EnumerateLifetimeSemanticsRValue) {
   // With an rvalue, it should not be destroyed until the end of the scope.
   int Copies = 0;
   int Moves = 0;
   int Destructors = 0;
   {
-    auto E1 = enumerate(Range<true, false>(Copies, Moves, Destructors));
+    Range<true, false> R(Copies, Moves, Destructors);
+    {
+      auto E = enumerate(std::move(R));
+      (void)E;
       // Doesn't compile.  rvalue ranges must be moveable.
       // auto E2 = enumerate(Range<false, true>(Copies, Moves, Destructors));
       EXPECT_EQ(0, Copies);
       EXPECT_EQ(1, Moves);
+      EXPECT_EQ(0, Destructors);
+    }
+    EXPECT_EQ(0, Copies);
+    EXPECT_EQ(1, Moves);
     EXPECT_EQ(1, Destructors);
   }
   EXPECT_EQ(0, Copies);
   EXPECT_EQ(1, Moves);
   EXPECT_EQ(2, Destructors);
+}
 
-  Copies = Moves = Destructors = 0;
+TEST(STLExtrasTest, EnumerateLifetimeSemanticsLValue) {
   // With an lvalue, it should not be destroyed even after the end of the scope.
   // lvalue ranges need be neither copyable nor moveable.
+  int Copies = 0;
+  int Moves = 0;
+  int Destructors = 0;
+  {
     Range<false, false> R(Copies, Moves, Destructors);
     {
-    auto Enumerator = enumerate(R);
-    (void)Enumerator;
+      auto E = enumerate(R);
+      (void)E;
       EXPECT_EQ(0, Copies);
       EXPECT_EQ(0, Moves);
       EXPECT_EQ(0, Destructors);
     }
     EXPECT_EQ(0, Copies);
     EXPECT_EQ(0, Moves);
     EXPECT_EQ(0, Destructors);
   }
+  EXPECT_EQ(0, Copies);
+  EXPECT_EQ(0, Moves);
+  EXPECT_EQ(1, Destructors);
+}
 
 TEST(STLExtrasTest, ApplyTuple) {
   auto T = std::make_tuple(1, 3, 7);
   auto U = llvm::apply_tuple(
       [](int A, int B, int C) { return std::make_tuple(A - B, B - C, C - A); },
       T);
 
   EXPECT_EQ(-2, std::get<0>(U));