SERVER-33628 Handle a mongo/unittest TODO c++11

    Use unique_ptr instead of shared_ptr.
    Simplify with range-based for loops and auto where appropriate.
    Change ASSERT_EQ guts to rely less on macros.
    Change the reserved _[A-Z] names (use trailing underscore instead).
    Change a vector<T*> to vector<unique_ptr<T>>.
    Change shared_ptr<T*> to unique_ptr<T>.
    Add a comment to mongoutils::str::stream lamenting missing relops.
    Add static to comparator() members.
    Add static constexpr to name() members.

2 files changed, 98 insertions, 79 deletions

diff --git a/src/mongo/unittest/unittest.cpp b/src/mongo/unittest/unittest.cpp
index 3dbb9670dc5..fa5f177da61 100644
--- a/src/mongo/unittest/unittest.cpp
+++ b/src/mongo/unittest/unittest.cpp
@@ -51,10 +51,6 @@
 #include "mongo/util/timer.h"
 
 namespace mongo {
-
-using std::shared_ptr;
-using std::string;
-
 namespace unittest {
 
 namespace {
@@ -108,8 +104,8 @@ public:
                 _millis / 1000.0);
         ss << result;
 
-        for (std::vector<std::string>::iterator i = _messages.begin(); i != _messages.end(); i++) {
-            ss << "\t" << *i << '\n';
+        for (const auto& i : _messages) {
+            ss << "\t" << i << '\n';
         }
 
         return ss.str();
@@ -119,7 +115,7 @@ public:
         return _rc;
     }
 
-    string _name;
+    std::string _name;
 
     int _rc;
     int _tests;
@@ -262,7 +258,7 @@ Suite::Suite(const std::string& name) : _name(name) {
 Suite::~Suite() {}
 
 void Suite::add(const std::string& name, const TestFunction& testFn) {
-    _tests.push_back(std::shared_ptr<TestHolder>(new TestHolder(name, testFn)));
+    _tests.push_back(stdx::make_unique<TestHolder>(name, testFn));
 }
 
 Result* Suite::run(const std::string& filter, int runsPerTest) {
@@ -274,9 +270,7 @@ Result* Suite::run(const std::string& filter, int runsPerTest) {
     Result* r = new Result(_name);
     Result::cur = r;
 
-    for (std::vector<std::shared_ptr<TestHolder>>::iterator i = _tests.begin(); i != _tests.end();
-         i++) {
-        std::shared_ptr<TestHolder>& tc = *i;
+    for (const auto& tc : _tests) {
         if (filter.size() && tc->getName().find(filter) == std::string::npos) {
             LOG(1) << "\t skipping test: " << tc->getName() << " because doesn't match filter"
                    << std::endl;
@@ -345,20 +339,19 @@ int Suite::run(const std::vector<std::string>& suites, const std::string& filter
     std::vector<std::string> torun(suites);
 
     if (torun.empty()) {
-        for (SuiteMap::const_iterator i = _allSuites().begin(); i != _allSuites().end(); ++i) {
-            torun.push_back(i->first);
+        for (const auto& kv : _allSuites()) {
+            torun.push_back(kv.first);
         }
     }
 
-    std::vector<Result*> results;
+    std::vector<std::unique_ptr<Result>> results;
 
-    for (std::vector<std::string>::iterator i = torun.begin(); i != torun.end(); i++) {
-        std::string name = *i;
+    for (std::string name : torun) {
         std::shared_ptr<Suite>& s = _allSuites()[name];
         fassert(16145, s != NULL);
 
         log() << "going to run suite: " << name << std::endl;
-        results.push_back(s->run(filter, runsPerTest));
+        results.emplace_back(s->run(filter, runsPerTest));
     }
 
     log() << "**************************************************" << std::endl;
@@ -373,8 +366,7 @@ int Suite::run(const std::vector<std::string>& suites, const std::string& filter
     std::vector<std::string> failedSuites;
 
     Result::cur = NULL;
-    for (std::vector<Result*>::iterator i = results.begin(); i != results.end(); i++) {
-        Result* r = *i;
+    for (const auto& r : results) {
         log() << r->toString();
         if (abs(r->rc()) > abs(rc))
             rc = r->rc();
@@ -382,18 +374,14 @@ int Suite::run(const std::vector<std::string>& suites, const std::string& filter
         tests += r->_tests;
         if (!r->_fails.empty()) {
             failedSuites.push_back(r->toString());
-            for (std::vector<std::string>::const_iterator j = r->_fails.begin();
-                 j != r->_fails.end();
-                 j++) {
-                const std::string& s = (*j);
+            for (const std::string& s : r->_fails) {
                 totals._fails.push_back(r->_name + "/" + s);
             }
         }
         asserts += r->_asserts;
         millis += r->_millis;
-
-        delete r;
     }
+    results.clear();
 
     totals._tests = tests;
     totals._asserts = asserts;
@@ -404,10 +392,7 @@ int Suite::run(const std::vector<std::string>& suites, const std::string& filter
     // summary
     if (!totals._fails.empty()) {
         log() << "Failing tests:" << std::endl;
-        for (std::vector<std::string>::const_iterator i = totals._fails.begin();
-             i != totals._fails.end();
-             i++) {
-            const std::string& s = (*i);
+        for (const std::string& s : totals._fails) {
             log() << "\t " << s << " Failed";
         }
         log() << "FAILURE - " << totals._fails.size() << " tests in " << failedSuites.size()
@@ -488,8 +473,8 @@ std::ostream& TestAssertionFailure::stream() {
 
 std::vector<std::string> getAllSuiteNames() {
     std::vector<std::string> result;
-    for (SuiteMap::const_iterator i = _allSuites().begin(); i != _allSuites().end(); ++i) {
-        result.push_back(i->first);
+    for (const auto& kv : _allSuites()) {
+        result.push_back(kv.first);
     }
     return result;
 }
diff --git a/src/mongo/unittest/unittest.h b/src/mongo/unittest/unittest.h
index aef46ac464f..44b1e68846c 100644
--- a/src/mongo/unittest/unittest.h
+++ b/src/mongo/unittest/unittest.h
@@ -37,6 +37,7 @@
 #include <cmath>
 #include <sstream>
 #include <string>
+#include <type_traits>
 #include <utility>
 #include <vector>
 
@@ -89,19 +90,19 @@
 #define ASSERT_LESS_THAN_OR_EQUALS(a, b) ASSERT_LTE(a, b)
 #define ASSERT_GREATER_THAN_OR_EQUALS(a, b) ASSERT_GTE(a, b)
 
-#define ASSERT_EQ(a, b) _ASSERT_COMPARISON(EQ, a, b)
-#define ASSERT_NE(a, b) _ASSERT_COMPARISON(NE, a, b)
-#define ASSERT_LT(a, b) _ASSERT_COMPARISON(LT, a, b)
-#define ASSERT_LTE(a, b) _ASSERT_COMPARISON(LTE, a, b)
-#define ASSERT_GT(a, b) _ASSERT_COMPARISON(GT, a, b)
-#define ASSERT_GTE(a, b) _ASSERT_COMPARISON(GTE, a, b)
+#define ASSERT_EQ(a, b) ASSERT_COMPARISON_(kEq, a, b)
+#define ASSERT_NE(a, b) ASSERT_COMPARISON_(kNe, a, b)
+#define ASSERT_LT(a, b) ASSERT_COMPARISON_(kLt, a, b)
+#define ASSERT_LTE(a, b) ASSERT_COMPARISON_(kLe, a, b)
+#define ASSERT_GT(a, b) ASSERT_COMPARISON_(kGt, a, b)
+#define ASSERT_GTE(a, b) ASSERT_COMPARISON_(kGe, a, b)
 
 /**
  * Binary comparison utility macro.  Do not use directly.
  */
-#define _ASSERT_COMPARISON(COMPARISON, a, b)                                                       \
-    if (::mongo::unittest::ComparisonAssertion_##COMPARISON ca =                                   \
-            ::mongo::unittest::ComparisonAssertion_##COMPARISON(__FILE__, __LINE__, #a, #b, a, b)) \
+#define ASSERT_COMPARISON_(OP, a, b)                                                           \
+    if (auto ca = ::mongo::unittest::ComparisonAssertion<::mongo::unittest::ComparisonOp::OP>( \
+            __FILE__, __LINE__, #a, #b, a, b))                                                 \
     ca.failure().stream()
 
 /**
@@ -421,8 +422,7 @@ protected:
     virtual void setupTests();
 
 private:
-    // TODO(C++11): Make this hold unique_ptrs.
-    typedef std::vector<std::shared_ptr<TestHolder>> TestHolderList;
+    typedef std::vector<std::unique_ptr<TestHolder>> TestHolderList;
 
     std::string _name;
     TestHolderList _tests;
@@ -506,45 +506,79 @@ private:
     bool _enabled;
 };
 
-#define DECLARE_COMPARISON_ASSERTION(NAME, OPERATOR)                                          \
-    class ComparisonAssertion_##NAME {                                                        \
-        typedef void (ComparisonAssertion_##NAME::*bool_type)() const;                        \
-                                                                                              \
-    public:                                                                                   \
-        template <typename A, typename B>                                                     \
-        ComparisonAssertion_##NAME(const std::string& theFile,                                \
-                                   unsigned theLine,                                          \
-                                   StringData aExpression,                                    \
-                                   StringData bExpression,                                    \
-                                   const A& a,                                                \
-                                   const B& b) {                                              \
-            if (a OPERATOR b) {                                                               \
-                return;                                                                       \
-            }                                                                                 \
-            std::ostringstream os;                                                            \
-            os << "Expected " << aExpression << " " #OPERATOR " " << bExpression << " (" << a \
-               << " " #OPERATOR " " << b << ")";                                              \
-            _assertion.reset(new TestAssertionFailure(theFile, theLine, os.str()));           \
-        }                                                                                     \
-        operator bool_type() const {                                                          \
-            return _assertion.get() ? &ComparisonAssertion_##NAME::comparison_failed : NULL;  \
-        }                                                                                     \
-        TestAssertionFailure failure() {                                                      \
-            return *_assertion;                                                               \
-        }                                                                                     \
-                                                                                              \
-    private:                                                                                  \
-        void comparison_failed() const {}                                                     \
-        std::shared_ptr<TestAssertionFailure> _assertion;                                     \
+enum class ComparisonOp { kEq, kNe, kLt, kLe, kGt, kGe };
+
+template <ComparisonOp op>
+class ComparisonAssertion {
+private:
+    template <ComparisonOp val>
+    using OpTag = std::integral_constant<ComparisonOp, val>;
+
+    static auto comparator(OpTag<ComparisonOp::kEq>) {
+        return [](auto&& a, auto&& b) { return a == b; };
+    }
+    static auto comparator(OpTag<ComparisonOp::kNe>) {
+        return [](auto&& a, auto&& b) { return a != b; };
+    }
+    static auto comparator(OpTag<ComparisonOp::kLt>) {
+        return [](auto&& a, auto&& b) { return a < b; };
+    }
+    static auto comparator(OpTag<ComparisonOp::kLe>) {
+        return [](auto&& a, auto&& b) { return a <= b; };
+    }
+    static auto comparator(OpTag<ComparisonOp::kGt>) {
+        return [](auto&& a, auto&& b) { return a > b; };
+    }
+    static auto comparator(OpTag<ComparisonOp::kGe>) {
+        return [](auto&& a, auto&& b) { return a >= b; };
+    }
+
+    static constexpr StringData name(OpTag<ComparisonOp::kEq>) {
+        return "=="_sd;
+    }
+    static constexpr StringData name(OpTag<ComparisonOp::kNe>) {
+        return "!="_sd;
+    }
+    static constexpr StringData name(OpTag<ComparisonOp::kLt>) {
+        return "<"_sd;
+    }
+    static constexpr StringData name(OpTag<ComparisonOp::kLe>) {
+        return "<="_sd;
+    }
+    static constexpr StringData name(OpTag<ComparisonOp::kGt>) {
+        return ">"_sd;
+    }
+    static constexpr StringData name(OpTag<ComparisonOp::kGe>) {
+        return ">="_sd;
     }
 
-DECLARE_COMPARISON_ASSERTION(EQ, ==);
-DECLARE_COMPARISON_ASSERTION(NE, !=);
-DECLARE_COMPARISON_ASSERTION(LT, <);
-DECLARE_COMPARISON_ASSERTION(LTE, <=);
-DECLARE_COMPARISON_ASSERTION(GT, >);
-DECLARE_COMPARISON_ASSERTION(GTE, >=);
-#undef DECLARE_COMPARISON_ASSERTION
+public:
+    template <typename A, typename B>
+    ComparisonAssertion(const std::string& theFile,
+                        unsigned theLine,
+                        StringData aExpression,
+                        StringData bExpression,
+                        const A& a,
+                        const B& b) {
+        if (comparator(OpTag<op>{})(a, b)) {
+            return;
+        }
+        std::ostringstream os;
+        StringData opName = name(OpTag<op>{});
+        os << "Expected " << aExpression << " " << opName << " " << bExpression << " (" << a << " "
+           << opName << " " << b << ")";
+        _assertion = std::make_unique<TestAssertionFailure>(theFile, theLine, os.str());
+    }
+    explicit operator bool() const {
+        return static_cast<bool>(_assertion);
+    }
+    TestAssertionFailure failure() {
+        return *_assertion;
+    }
+
+private:
+    std::unique_ptr<TestAssertionFailure> _assertion;
+};
 
 /**
  * Get the value out of a StatusWith<T>, or throw an exception if it is not OK.