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

[ldionne]

Summary

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.

[• Quuxplusone]

libcxx/test/libcxx/algorithms/debug_less.pass.cpp

8–24

I recommend more like this. (And likewise throughout.)

[ldionne]

Make this change independent of D121123.

[ldionne]

CI passed after retrying the job that had failed due to a flake.

Landing this since this is now an uncontroversial workaround removal -- this is not built on top of the projected new assertion handler work.

libcxx/docs/DesignDocs/DebugMode.rst

 
 The various levels of checking provided by the debug mode follow.
 
 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``)
 --------------------------------------------------
 Defining ``_LIBCPP_DEBUG`` to ``1`` enables "iterator debugging", which provides
 additional assertions about the validity of iterators used by the program.
 
 The following containers and classes support iterator debugging:
 

libcxx/include/__algorithm/comp_ref_type.h

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef _LIBCPP___ALGORITHM_COMP_REF_TYPE_H
 #define _LIBCPP___ALGORITHM_COMP_REF_TYPE_H
 
-#include <__assert>
 #include <__config>
 #include <__debug>
 #include <__utility/declval.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
 
struct __debug_less
     }
 
     template <class _LHS, class _RHS>
     _LIBCPP_CONSTEXPR_AFTER_CXX11
     inline _LIBCPP_INLINE_VISIBILITY
     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;
     }
 
     template <class _LHS, class _RHS>
     _LIBCPP_CONSTEXPR_AFTER_CXX11
     inline _LIBCPP_INLINE_VISIBILITY
     void __do_compare_assert(long, _LHS &, _RHS &) {}
 };
 
 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
 };
 
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___ALGORITHM_COMP_REF_TYPE_H

libcxx/include/__assert

 #include <__config>
 #include <iosfwd> // for std::string
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #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
 
 struct _LIBCPP_TEMPLATE_VIS __libcpp_debug_info {
   _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
   __libcpp_debug_info()
       : __file_(nullptr), __line_(-1), __pred_(nullptr), __msg_(nullptr) {}

libcxx/test/libcxx/algorithms/debug_less.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-// 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 {
     int value;
     explicit MyType(int xvalue = 0) : value(xvalue) {}
 };
 
 template <int ID1, int ID2>
MT1 four(4);
 
         assert(d(one, three) == true);
         assert(called == 1);
         called = 0;
     }
 }
 
 void test_failing() {
-    int& called = CompareBase::called;
-    called = 0;
     MT0 one(1);
     MT0 two(2);
 
     {
         typedef BadComparator<MT0> C;
         typedef __debug_less<C> D;
         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");
     }
 }
 
 template <int>
 struct Tag {
   explicit Tag(int v) : value(v) {}
   int value;
 };

libcxx/test/libcxx/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/deallocate.assert.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // UNSUPPORTED: c++03
 
 // <experimental/memory_resource>
 
 // template <class T> class polymorphic_allocator
 
 // 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>
 
 #include "test_memory_resource.h"
 
 #include "test_macros.h"
+#include "debug_macros.h"
 
 namespace ex = std::experimental::pmr;
 
 int main(int, char**)
 {
     using Alloc = ex::polymorphic_allocator<int>;
     using Traits = std::allocator_traits<Alloc>;
     NullResource R;
     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;
 }

libcxx/test/libcxx/experimental/memory/memory.resource.adaptor/memory.resource.adaptor.mem/deallocate.assert.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // UNSUPPORTED: c++03
 
 // <experimental/memory_resource>
 
 // template <class T> class polymorphic_allocator
 
 // 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>
 
 #include "test_memory_resource.h"
 
 #include "test_macros.h"
+#include "debug_macros.h"
 
 namespace ex = std::experimental::pmr;
 
 int main(int, char**)
 {
     using Alloc = NullAllocator<char>;
 
     AllocController P;
     ex::resource_adaptor<Alloc> r(Alloc{P});
     ex::memory_resource & m1 = r;
 
     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;
 }

libcxx/utils/libcxx/test/format.py

def getTestsInDirectory(self, testSuite, pathInSuite, litConfig, localConfig):
             if filename.startswith('.') or filename in localConfig.excludes:
                 continue
 
             filepath = os.path.join(sourcePath, filename)
             if not os.path.isdir(filepath):
                 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)
         elif filename.endswith('.compile.pass.cpp'):
             steps = [
                 "%dbg(COMPILED WITH) %{cxx} %s %{flags} %{compile_flags} -fsyntax-only"
             ]
             return self._executeShTest(test, litConfig, steps)