[libc++] Remove workarounds for re-defining _LIBCPP_ASSERT in the test suite

As a fly-by fix, enable the complexity-changing assertions in __debug_less
only when the full debug mode is enabled, since debugging level 0 is usually
understood to only contain basic assertions that do not change the complexity
of algorithms.

Differential Revision: https://reviews.llvm.org/D121129

7 files changed, 31 insertions, 73 deletions

diff --git a/libcxx/docs/DesignDocs/DebugMode.rst b/libcxx/docs/DesignDocs/DebugMode.rst
index faf7b8e1b81c..b7922513fa91 100644
--- a/libcxx/docs/DesignDocs/DebugMode.rst
+++ b/libcxx/docs/DesignDocs/DebugMode.rst
@@ -27,18 +27,13 @@ No debugging checks (``_LIBCPP_DEBUG`` not defined)
 When ``_LIBCPP_DEBUG`` is not defined, there are no debugging checks performed by
 the library. This is the default.
 
-Basic checks (``_LIBCPP_DEBUG == 0``)
--------------------------------------
-When ``_LIBCPP_DEBUG`` is defined to ``0`` (to be understood as level ``0``), some
-debugging checks are enabled. The non-exhaustive list of things is:
-
-- Many algorithms, such as ``binary_search``, ``merge``, ``next_permutation``, and ``sort``,
-  wrap the user-provided comparator to assert that `!comp(y, x)` whenever
-  `comp(x, y)`. This can cause the user-provided comparator to be evaluated
-  up to twice as many times as it would be without ``_LIBCPP_DEBUG``, and
-  causes the library to violate some of the Standard's complexity clauses.
-
-- FIXME: Update this list
+Comparator consistency checks (``_LIBCPP_DEBUG == 1``)
+------------------------------------------------------
+Libc++ provides some checks for the consistency of comparators passed to algorithms. Specifically,
+many algorithms such as ``binary_search``, ``merge``, ``next_permutation``, and ``sort``, wrap the
+user-provided comparator to assert that `!comp(y, x)` whenever `comp(x, y)`. This can cause the
+user-provided comparator to be evaluated up to twice as many times as it would be without the
+debug mode, and causes the library to violate some of the Standard's complexity clauses.
 
 Iterator debugging checks (``_LIBCPP_DEBUG == 1``)
 --------------------------------------------------
diff --git a/libcxx/include/__algorithm/comp_ref_type.h b/libcxx/include/__algorithm/comp_ref_type.h
index ed0ed5904a44..c94c9634170c 100644
--- a/libcxx/include/__algorithm/comp_ref_type.h
+++ b/libcxx/include/__algorithm/comp_ref_type.h
@@ -9,7 +9,6 @@
 #ifndef _LIBCPP___ALGORITHM_COMP_REF_TYPE_H
 #define _LIBCPP___ALGORITHM_COMP_REF_TYPE_H
 
-#include <__assert>
 #include <__config>
 #include <__debug>
 #include <__utility/declval.h>
@@ -53,7 +52,7 @@ struct __debug_less
     decltype((void)declval<_Compare&>()(
         declval<_LHS &>(), declval<_RHS &>()))
     __do_compare_assert(int, _LHS & __l, _RHS & __r) {
-        _LIBCPP_ASSERT(!__comp_(__l, __r),
+        _LIBCPP_DEBUG_ASSERT(!__comp_(__l, __r),
             "Comparator does not induce a strict weak ordering");
         (void)__l;
         (void)__r;
@@ -69,10 +68,10 @@ template <class _Comp>
 struct __comp_ref_type {
   // Pass the comparator by lvalue reference. Or in debug mode, using a
   // debugging wrapper that stores a reference.
-#if _LIBCPP_DEBUG_LEVEL == 0
-  typedef _Comp& type;
-#else
+#if _LIBCPP_DEBUG_LEVEL == 2
   typedef __debug_less<_Comp> type;
+#else
+  typedef _Comp& type;
 #endif
 };
 
diff --git a/libcxx/include/__assert b/libcxx/include/__assert
index d51512dcf379..75763d037415 100644
--- a/libcxx/include/__assert
+++ b/libcxx/include/__assert
@@ -18,16 +18,9 @@
 #endif
 
 #if _LIBCPP_DEBUG_LEVEL >= 1
-#   define _LIBCPP_ASSERT_IMPL(x, m) ((x) ? (void)0 : ::std::__libcpp_debug_function(::std::__libcpp_debug_info(__FILE__, __LINE__, #x, m)))
+#   define _LIBCPP_ASSERT(x, m) ((x) ? (void)0 : ::std::__libcpp_debug_function(::std::__libcpp_debug_info(__FILE__, __LINE__, #x, m)))
 #else
-#   define _LIBCPP_ASSERT_IMPL(x, m) ((void)0)
-#endif
-
-// We do this dance because some of our tests re-define _LIBCPP_ASSERT to something else.
-// In the future, we should find other ways to test our assertions and disallow re-defining
-// _LIBCPP_ASSERT.
-#if !defined(_LIBCPP_ASSERT)
-#   define _LIBCPP_ASSERT(x, m) _LIBCPP_ASSERT_IMPL(x, m)
+#   define _LIBCPP_ASSERT(x, m) ((void)0)
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD
diff --git a/libcxx/test/libcxx/algorithms/debug_less.pass.cpp b/libcxx/test/libcxx/algorithms/debug_less.pass.cpp
index 932a074560ce..5a45ebabaa80 100644
--- a/libcxx/test/libcxx/algorithms/debug_less.pass.cpp
+++ b/libcxx/test/libcxx/algorithms/debug_less.pass.cpp
@@ -6,25 +6,22 @@
 //
 //===----------------------------------------------------------------------===//
 
-// UNSUPPORTED: no-exceptions
-
 // <algorithm>
 
 // template <class _Compare> struct __debug_less
 
 // __debug_less checks that a comparator actually provides a strict-weak ordering.
 
-struct DebugException {};
+// UNSUPPORTED: libcxx-no-debug-mode
 
-// ADDITIONAL_COMPILE_FLAGS: -Wno-macro-redefined
-#define _LIBCPP_DEBUG 0
-#define _LIBCPP_ASSERT(x, m) ((x) ? (void)0 : throw ::DebugException())
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DEBUG=1
 
 #include <algorithm>
 #include <iterator>
 #include <cassert>
 
 #include "test_macros.h"
+#include "debug_macros.h"
 
 template <int ID>
 struct MyType {
@@ -142,8 +139,6 @@ void test_passing() {
 }
 
 void test_failing() {
-    int& called = CompareBase::called;
-    called = 0;
     MT0 one(1);
     MT0 two(2);
 
@@ -153,14 +148,7 @@ void test_failing() {
         C c;
         D d(c);
 
-        try {
-            d(one, two);
-            assert(false);
-        } catch (DebugException const&) {
-        }
-
-        assert(called == 2);
-        called = 0;
+        TEST_LIBCPP_ASSERT_FAILURE(d(one, two), "Comparator does not induce a strict weak ordering");
     }
 }
 
diff --git a/libcxx/test/libcxx/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/db_deallocate.pass.cpp b/libcxx/test/libcxx/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/deallocate.assert.pass.cpp
index 35e887d22d4c..af6115cc7b13 100644
--- a/libcxx/test/libcxx/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/db_deallocate.pass.cpp
+++ b/libcxx/test/libcxx/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/deallocate.assert.pass.cpp
@@ -14,11 +14,10 @@
 
 // T* polymorphic_allocator<T>::deallocate(T*, size_t size)
 
-int AssertCount = 0;
+// UNSUPPORTED: libcxx-no-debug-mode
+
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DEBUG=1
 
-// ADDITIONAL_COMPILE_FLAGS: -Wno-macro-redefined
-#define _LIBCPP_ASSERT(x, m) ((x) ? (void)0 : (void)::AssertCount++)
-#define _LIBCPP_DEBUG 0
 #include <experimental/memory_resource>
 #include <type_traits>
 #include <cassert>
@@ -26,6 +25,7 @@ int AssertCount = 0;
 #include "test_memory_resource.h"
 
 #include "test_macros.h"
+#include "debug_macros.h"
 
 namespace ex = std::experimental::pmr;
 
@@ -37,10 +37,8 @@ int main(int, char**)
     Alloc a(&R);
     const std::size_t maxSize = Traits::max_size(a);
 
-    a.deallocate(nullptr, maxSize);
-    assert(AssertCount == 0);
-    a.deallocate(nullptr, maxSize + 1);
-    assert(AssertCount == 1);
+    a.deallocate(nullptr, maxSize); // no assertion
+    TEST_LIBCPP_ASSERT_FAILURE(a.deallocate(nullptr, maxSize + 1), "deallocate called for size which exceeds max_size()");
 
-  return 0;
+    return 0;
 }
diff --git a/libcxx/test/libcxx/experimental/memory/memory.resource.adaptor/memory.resource.adaptor.mem/db_deallocate.pass.cpp b/libcxx/test/libcxx/experimental/memory/memory.resource.adaptor/memory.resource.adaptor.mem/deallocate.assert.pass.cpp
index 850d5aa0363b..3665729ee46b 100644
--- a/libcxx/test/libcxx/experimental/memory/memory.resource.adaptor/memory.resource.adaptor.mem/db_deallocate.pass.cpp
+++ b/libcxx/test/libcxx/experimental/memory/memory.resource.adaptor/memory.resource.adaptor.mem/deallocate.assert.pass.cpp
@@ -14,11 +14,10 @@
 
 // T* polymorphic_allocator<T>::deallocate(T*, size_t size)
 
-int AssertCount = 0;
+// UNSUPPORTED: libcxx-no-debug-mode
+
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DEBUG=1
 
-// ADDITIONAL_COMPILE_FLAGS: -Wno-macro-redefined
-#define _LIBCPP_ASSERT(x, m) ((x) ? (void)0 : (void)::AssertCount++)
-#define _LIBCPP_DEBUG 0
 #include <experimental/memory_resource>
 #include <type_traits>
 #include <cassert>
@@ -26,6 +25,7 @@ int AssertCount = 0;
 #include "test_memory_resource.h"
 
 #include "test_macros.h"
+#include "debug_macros.h"
 
 namespace ex = std::experimental::pmr;
 
@@ -40,10 +40,8 @@ int main(int, char**)
     std::size_t maxSize = std::numeric_limits<std::size_t>::max()
                             - alignof(std::max_align_t);
 
-    m1.deallocate(nullptr, maxSize);
-    assert(AssertCount == 0);
-    m1.deallocate(nullptr, maxSize + 1);
-    assert(AssertCount >= 1);
+    m1.deallocate(nullptr, maxSize); // no assertion
+    TEST_LIBCPP_ASSERT_FAILURE(m1.deallocate(nullptr, maxSize + 1), "do_deallocate called for size which exceeds the maximum allocation size");
 
-  return 0;
+    return 0;
 }
diff --git a/libcxx/utils/libcxx/test/format.py b/libcxx/utils/libcxx/test/format.py
index b53dfdc13334..1892c5fcd371 100644
--- a/libcxx/utils/libcxx/test/format.py
+++ b/libcxx/utils/libcxx/test/format.py
@@ -194,24 +194,11 @@ class CxxStandardLibraryTest(lit.formats.TestFormat):
                 if any([re.search(ext, filename) for ext in SUPPORTED_SUFFIXES]):
                     yield lit.Test.Test(testSuite, pathInSuite + (filename,), localConfig)
 
-    def _disableWithModules(self, test):
-        with open(test.getSourcePath(), 'rb') as f:
-            contents = f.read()
-        return b'#define _LIBCPP_ASSERT' in contents
-
     def execute(self, test, litConfig):
         VERIFY_FLAGS = '-Xclang -verify -Xclang -verify-ignore-unexpected=note -ferror-limit=0'
         supportsVerify = 'verify-support' in test.config.available_features
         filename = test.path_in_suite[-1]
 
-        # TODO(ldionne): We currently disable tests that re-define _LIBCPP_ASSERT
-        #                when we run with modules enabled. Instead, we should
-        #                split the part that does a death test outside of the
-        #                test, and only disable that part when modules are
-        #                enabled.
-        if 'modules-build' in test.config.available_features and self._disableWithModules(test):
-            return lit.Test.Result(lit.Test.UNSUPPORTED, 'Test {} is unsupported when modules are enabled')
-
         if re.search('[.]sh[.][^.]+$', filename):
             steps = [ ] # The steps are already in the script
             return self._executeShTest(test, litConfig, steps)