[libcxx-commits] [libcxx] 95c0f2d - [libc++] Remove workarounds for re-defining _LIBCPP_ASSERT in the test suite
Louis Dionne via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Mar 8 07:41:45 PST 2022
Author: Louis Dionne
Date: 2022-03-08T10:41:38-05:00
New Revision: 95c0f2d115e29e3429226cccc1209f5640dd1b6a
URL: https://github.com/llvm/llvm-project/commit/95c0f2d115e29e3429226cccc1209f5640dd1b6a
DIFF: https://github.com/llvm/llvm-project/commit/95c0f2d115e29e3429226cccc1209f5640dd1b6a.diff
LOG: [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
Added:
libcxx/test/libcxx/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/deallocate.assert.pass.cpp
libcxx/test/libcxx/experimental/memory/memory.resource.adaptor/memory.resource.adaptor.mem/deallocate.assert.pass.cpp
Modified:
libcxx/docs/DesignDocs/DebugMode.rst
libcxx/include/__algorithm/comp_ref_type.h
libcxx/include/__assert
libcxx/test/libcxx/algorithms/debug_less.pass.cpp
libcxx/utils/libcxx/test/format.py
Removed:
libcxx/test/libcxx/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/db_deallocate.pass.cpp
libcxx/test/libcxx/experimental/memory/memory.resource.adaptor/memory.resource.adaptor.mem/db_deallocate.pass.cpp
################################################################################
diff --git a/libcxx/docs/DesignDocs/DebugMode.rst b/libcxx/docs/DesignDocs/DebugMode.rst
index faf7b8e1b81cd..b7922513fa91b 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 ed0ed5904a44c..c94c9634170cb 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 d51512dcf379c..75763d0374156 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 932a074560ce7..5a45ebabaa806 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
similarity index 75%
rename from libcxx/test/libcxx/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/db_deallocate.pass.cpp
rename to libcxx/test/libcxx/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/deallocate.assert.pass.cpp
index 35e887d22d4c6..af6115cc7b135 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
similarity index 75%
rename from libcxx/test/libcxx/experimental/memory/memory.resource.adaptor/memory.resource.adaptor.mem/db_deallocate.pass.cpp
rename to libcxx/test/libcxx/experimental/memory/memory.resource.adaptor/memory.resource.adaptor.mem/deallocate.assert.pass.cpp
index 850d5aa0363be..3665729ee46bb 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 b53dfdc133346..1892c5fcd3710 100644
--- a/libcxx/utils/libcxx/test/format.py
+++ b/libcxx/utils/libcxx/test/format.py
@@ -194,24 +194,11 @@ def getTestsInDirectory(self, testSuite, pathInSuite, litConfig, localConfig):
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)
More information about the libcxx-commits
mailing list