[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

9–30

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

libcxx/include/__assert

 # error "_LIBCPP_ENABLE_ASSERTIONS must be set to 0 or 1"
 #endif
 
 #ifndef _LIBCPP_ASSERTION_HANDLER
 # define _LIBCPP_ASSERTION_HANDLER ::std::__libcpp_default_assertion_handler
 #endif
 
 #if _LIBCPP_ENABLE_ASSERTIONS
-# define _LIBCPP_ASSERT_IMPL(x, m) ((x) ? (void)0 : _LIBCPP_ASSERTION_HANDLER(::std::__libcpp_assertion_info(__FILE__, __LINE__, #x, m)))
+# define _LIBCPP_ASSERT(x, m) ((x) ? (void)0 : _LIBCPP_ASSERTION_HANDLER(::std::__libcpp_assertion_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
 
 //////////////////////////////////////////////////////////////////
 // For backwards compatibility with clients of the Debug mode
 struct _LIBCPP_TEMPLATE_VIS __libcpp_debug_info {
   _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR

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.
 
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ENABLE_ASSERTIONS=1
+// ADDITIONAL_COMPILE_FLAGS: -D'_LIBCPP_ASSERTION_HANDLER(x)=::assertion_handler(x)'
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DEBUG=1
+#include <__assert>
 struct DebugException {};
-
-// ADDITIONAL_COMPILE_FLAGS: -Wno-macro-redefined
-#define _LIBCPP_DEBUG 0
-#define _LIBCPP_ASSERT(x, m) ((x) ? (void)0 : throw ::DebugException())
+inline void assertion_handler(std::__libcpp_assertion_info) {
+    throw ::DebugException();
+}
 
 #include <algorithm>
 #include <iterator>
 #include <cassert>
 
 #include "test_macros.h"
 
 template <int ID>
 struct MyType {
     int value;
     explicit MyType(int xvalue = 0) : value(xvalue) {}
 };
 
 template <int ID1, int ID2>

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)
 
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ENABLE_ASSERTIONS=1
+// ADDITIONAL_COMPILE_FLAGS: -D'_LIBCPP_ASSERTION_HANDLER(x)=::assertion_handler(x)'
+#include <__assert>
 int AssertCount = 0;
+inline void assertion_handler(std::__libcpp_assertion_info) {
+  ++AssertCount;
+}
 
-// 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"
 

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)
 
+// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ENABLE_ASSERTIONS=1
+// ADDITIONAL_COMPILE_FLAGS: -D'_LIBCPP_ASSERTION_HANDLER(x)=::assertion_handler(x)'
+#include <__assert>
 int AssertCount = 0;
+inline void assertion_handler(std::__libcpp_assertion_info) {
+  ++AssertCount;
+}
 
-// 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"
 

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)