diff --git a/libcxx/docs/ReleaseNotes/22.rst b/libcxx/docs/ReleaseNotes/22.rst index 59990d1aa5feb..9f9927095a61f 100644 --- a/libcxx/docs/ReleaseNotes/22.rst +++ b/libcxx/docs/ReleaseNotes/22.rst @@ -117,6 +117,13 @@ Potentially breaking changes first element being returned from ``find`` will be broken, and ``lower_bound`` or ``equal_range`` should be used instead. +- The algorithms for ``std::{map,set}`` ``lower_bound`` and ``upper_bound`` operations were modified such that their + result changed for comparators that are not a strict weak order. Being a strict weak order was always a requirement + of the Standard and still is, however in this release libc++ changed the behavior of ``std::{map,set}`` for such + comparators. Since this may be tricky to work around in some cases, an escape hatch is provided in this release: + defining ``_LIBCPP_ENABLE_LEGACY_TREE_LOWER_UPPER_BOUND`` will revert to the historical implementation of + these operations. That escape hatch will be removed in LLVM 23. + - The ABI flag ``_LIBCPP_ABI_NO_REVERSE_ITERATOR_SECOND_MEMBER`` has been split off from ``_LIBCPP_ABI_NO_ITERATOR_BASES``. If you are using this flag and care about ABI stability, you should set ``_LIBCPP_ABI_NO_REVERSE_ITERATOR_SECOND_MEMBER`` as well. diff --git a/libcxx/include/__tree b/libcxx/include/__tree index 84711057be409..eb17f7d36936c 100644 --- a/libcxx/include/__tree +++ b/libcxx/include/__tree @@ -1247,24 +1247,74 @@ public: return __result; } + // Compatibility escape hatch for comparators that are not strict weak orderings. This + // can be removed for the LLVM 23 release. +#if defined(_LIBCPP_ENABLE_LEGACY_TREE_LOWER_UPPER_BOUND) + template + _LIBCPP_HIDE_FROM_ABI __end_node_pointer __lower_bound_unique_compat_impl(const _Key& __v) const { + auto __rt = __root(); + auto __result = __end_node(); + while (__rt != nullptr) { + if (!value_comp()(__rt->__get_value(), __v)) { + __result = std::__static_fancy_pointer_cast<__end_node_pointer>(__rt); + __rt = std::__static_fancy_pointer_cast<__node_pointer>(__rt->__left_); + } else { + __rt = std::__static_fancy_pointer_cast<__node_pointer>(__rt->__right_); + } + } + return __result; + } + + template + _LIBCPP_HIDE_FROM_ABI __end_node_pointer __upper_bound_unique_compat_impl(const _Key& __v) const { + auto __rt = __root(); + auto __result = __end_node(); + while (__rt != nullptr) { + if (value_comp()(__v, __rt->__get_value())) { + __result = std::__static_fancy_pointer_cast<__end_node_pointer>(__rt); + __rt = std::__static_fancy_pointer_cast<__node_pointer>(__rt->__left_); + } else { + __rt = std::__static_fancy_pointer_cast<__node_pointer>(__rt->__right_); + } + } + return __result; + } +#endif // _LIBCPP_ENABLE_LEGACY_TREE_LOWER_UPPER_BOUND + template _LIBCPP_HIDE_FROM_ABI iterator __lower_bound_unique(const _Key& __v) { +#if defined(_LIBCPP_ENABLE_LEGACY_TREE_LOWER_UPPER_BOUND) + return iterator(__lower_bound_unique_compat_impl(__v)); +#else return iterator(__lower_upper_bound_unique_impl(__v)); +#endif } template _LIBCPP_HIDE_FROM_ABI const_iterator __lower_bound_unique(const _Key& __v) const { +#if defined(_LIBCPP_ENABLE_LEGACY_TREE_LOWER_UPPER_BOUND) + return const_iterator(__lower_bound_unique_compat_impl(__v)); +#else return const_iterator(__lower_upper_bound_unique_impl(__v)); +#endif } template _LIBCPP_HIDE_FROM_ABI iterator __upper_bound_unique(const _Key& __v) { +#if defined(_LIBCPP_ENABLE_LEGACY_TREE_LOWER_UPPER_BOUND) + return iterator(__upper_bound_unique_compat_impl(__v)); +#else return iterator(__lower_upper_bound_unique_impl(__v)); +#endif } template _LIBCPP_HIDE_FROM_ABI const_iterator __upper_bound_unique(const _Key& __v) const { +#if defined(_LIBCPP_ENABLE_LEGACY_TREE_LOWER_UPPER_BOUND) + return iterator(__upper_bound_unique_compat_impl(__v)); +#else return iterator(__lower_upper_bound_unique_impl(__v)); +#endif } private: diff --git a/libcxx/include/__utility/lazy_synth_three_way_comparator.h b/libcxx/include/__utility/lazy_synth_three_way_comparator.h index 8c78742ccb4e3..906166bd2a61a 100644 --- a/libcxx/include/__utility/lazy_synth_three_way_comparator.h +++ b/libcxx/include/__utility/lazy_synth_three_way_comparator.h @@ -9,6 +9,7 @@ #ifndef _LIBCPP___UTILITY_LAZY_SYNTH_THREE_WAY_COMPARATOR_H #define _LIBCPP___UTILITY_LAZY_SYNTH_THREE_WAY_COMPARATOR_H +#include <__assert> #include <__config> #include <__type_traits/conjunction.h> #include <__type_traits/desugars_to.h> @@ -40,8 +41,19 @@ struct __lazy_compare_result { _LIBCPP_CTOR_LIFETIMEBOUND const _RHS& __rhs) : __comp_(__comp), __lhs_(__lhs), __rhs_(__rhs) {} - _LIBCPP_HIDE_FROM_ABI bool __less() const { return __comp_(__lhs_, __rhs_); } - _LIBCPP_HIDE_FROM_ABI bool __greater() const { return __comp_(__rhs_, __lhs_); } + _LIBCPP_HIDE_FROM_ABI bool __less() const { + bool __result = __comp_(__lhs_, __rhs_); + _LIBCPP_ASSERT_SEMANTIC_REQUIREMENT(__result ? !static_cast(__comp_(__rhs_, __lhs_)) : true, + "Comparator does not induce a strict weak ordering"); + return __result; + } + + _LIBCPP_HIDE_FROM_ABI bool __greater() const { + bool __result = __comp_(__rhs_, __lhs_); + _LIBCPP_ASSERT_SEMANTIC_REQUIREMENT(__result ? !static_cast(__comp_(__lhs_, __rhs_)) : true, + "Comparator does not induce a strict weak ordering"); + return __result; + } }; // This class provides three way comparison between _LHS and _RHS as efficiently as possible. This can be specialized if diff --git a/libcxx/test/libcxx/containers/associative/debug.non-strict-weak-ordering.pass.cpp b/libcxx/test/libcxx/containers/associative/debug.non-strict-weak-ordering.pass.cpp new file mode 100644 index 0000000000000..00a3a0730545d --- /dev/null +++ b/libcxx/test/libcxx/containers/associative/debug.non-strict-weak-ordering.pass.cpp @@ -0,0 +1,130 @@ +//===----------------------------------------------------------------------===// +// +// 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 +// +//===----------------------------------------------------------------------===// + +// +// + +// This test ensures that libc++ detects when std::set or std::map are used with a +// predicate that is not a strict weak ordering when the debug mode is enabled. + +// REQUIRES: libcpp-hardening-mode=debug +// UNSUPPORTED: c++03, c++11, c++14 + +#include +#include +#include + +#include "check_assertion.h" +#include "test_macros.h" + +struct InvalidLess { + bool operator()(int a, int b) const { + if (b == 0) + return true; + return static_cast(a % b); + } +}; + +int main() { + // std::set + { + // find + { + std::set s = {1, 2, 3, 4}; + TEST_LIBCPP_ASSERT_FAILURE(s.find(4), "Comparator does not induce a strict weak ordering"); + TEST_LIBCPP_ASSERT_FAILURE(std::as_const(s).find(4), "Comparator does not induce a strict weak ordering"); + } + + // upper_bound + { + std::set s = {1, 2, 3, 4}; + TEST_LIBCPP_ASSERT_FAILURE(s.upper_bound(4), "Comparator does not induce a strict weak ordering"); + TEST_LIBCPP_ASSERT_FAILURE(std::as_const(s).upper_bound(4), "Comparator does not induce a strict weak ordering"); + } + + // lower_bound + { + std::set s = {1, 2, 3, 4}; + TEST_LIBCPP_ASSERT_FAILURE(s.lower_bound(4), "Comparator does not induce a strict weak ordering"); + TEST_LIBCPP_ASSERT_FAILURE(std::as_const(s).lower_bound(4), "Comparator does not induce a strict weak ordering"); + } + + // equal_range + { + std::set s = {1, 2, 3, 4}; + TEST_LIBCPP_ASSERT_FAILURE(s.equal_range(4), "Comparator does not induce a strict weak ordering"); + TEST_LIBCPP_ASSERT_FAILURE(std::as_const(s).equal_range(4), "Comparator does not induce a strict weak ordering"); + } + + // count + { + std::set s = {1, 2, 3, 4}; + TEST_LIBCPP_ASSERT_FAILURE(s.count(4), "Comparator does not induce a strict weak ordering"); + TEST_LIBCPP_ASSERT_FAILURE(std::as_const(s).count(4), "Comparator does not induce a strict weak ordering"); + } + + // contains +#if TEST_STD_VER >= 20 + { + std::set s = {1, 2, 3, 4}; + TEST_LIBCPP_ASSERT_FAILURE(s.contains(4), "Comparator does not induce a strict weak ordering"); + TEST_LIBCPP_ASSERT_FAILURE(std::as_const(s).contains(4), "Comparator does not induce a strict weak ordering"); + } +#endif + } + + // std::map + { + using X = int; + X const x = 99; + + // find + { + std::map s = {{1, x}, {2, x}, {3, x}, {4, x}}; + TEST_LIBCPP_ASSERT_FAILURE(s.find(4), "Comparator does not induce a strict weak ordering"); + TEST_LIBCPP_ASSERT_FAILURE(std::as_const(s).find(4), "Comparator does not induce a strict weak ordering"); + } + + // upper_bound + { + std::map s = {{1, x}, {2, x}, {3, x}, {4, x}}; + TEST_LIBCPP_ASSERT_FAILURE(s.upper_bound(4), "Comparator does not induce a strict weak ordering"); + TEST_LIBCPP_ASSERT_FAILURE(std::as_const(s).upper_bound(4), "Comparator does not induce a strict weak ordering"); + } + + // lower_bound + { + std::map s = {{1, x}, {2, x}, {3, x}, {4, x}}; + TEST_LIBCPP_ASSERT_FAILURE(s.lower_bound(4), "Comparator does not induce a strict weak ordering"); + TEST_LIBCPP_ASSERT_FAILURE(std::as_const(s).lower_bound(4), "Comparator does not induce a strict weak ordering"); + } + + // equal_range + { + std::map s = {{1, x}, {2, x}, {3, x}, {4, x}}; + TEST_LIBCPP_ASSERT_FAILURE(s.equal_range(4), "Comparator does not induce a strict weak ordering"); + TEST_LIBCPP_ASSERT_FAILURE(std::as_const(s).equal_range(4), "Comparator does not induce a strict weak ordering"); + } + + // count + { + std::map s = {{1, x}, {2, x}, {3, x}, {4, x}}; + TEST_LIBCPP_ASSERT_FAILURE(s.count(4), "Comparator does not induce a strict weak ordering"); + TEST_LIBCPP_ASSERT_FAILURE(std::as_const(s).count(4), "Comparator does not induce a strict weak ordering"); + } + + // contains +#if TEST_STD_VER >= 20 + { + std::map s = {{1, x}, {2, x}, {3, x}, {4, x}}; + TEST_LIBCPP_ASSERT_FAILURE(s.contains(4), "Comparator does not induce a strict weak ordering"); + TEST_LIBCPP_ASSERT_FAILURE(std::as_const(s).contains(4), "Comparator does not induce a strict weak ordering"); + } +#endif + } +} diff --git a/libcxx/test/libcxx/containers/associative/lower_upper_bound_non_strict_weak_order.pass.cpp b/libcxx/test/libcxx/containers/associative/lower_upper_bound_non_strict_weak_order.pass.cpp new file mode 100644 index 0000000000000..a3e2f3bb631b3 --- /dev/null +++ b/libcxx/test/libcxx/containers/associative/lower_upper_bound_non_strict_weak_order.pass.cpp @@ -0,0 +1,151 @@ +//===----------------------------------------------------------------------===// +// +// 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 +// +//===----------------------------------------------------------------------===// + +// +// + +// This test ensures that libc++ maintains its historical behavior when std::set +// or std::map are used with a comparator that isn't quite a strict weak ordering +// when _LIBCPP_ENABLE_LEGACY_TREE_LOWER_UPPER_BOUND is defined. +// +// This escape hatch is only supported as a temporary means to give a bit more time +// for codebases to fix incorrect usages of associative containers. + +// This precise test reproduces a comparator similar to boost::icl::exclusive_less +// for discrete right-open intervals: it orders disjoint intervals but treats +// overlapping intervals as equivalent, which is incorrect but was relied upon. + +// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ENABLE_LEGACY_TREE_LOWER_UPPER_BOUND +// UNSUPPORTED: c++03, c++11, c++14 + +#include +#include +#include +#include + +struct Interval { + int lower; + int upper; +}; + +struct ExclusiveLess { + bool operator()(const Interval& lhs, const Interval& rhs) const { return lhs.upper <= rhs.lower; } +}; + +Interval right_open(int lo, int hi) { return Interval{lo, hi}; } + +int main() { + // std::set + { + // upper_bound + { + std::set intervals = { + right_open(0, 2), + right_open(3, 5), + right_open(6, 9), + right_open(10, 12), + }; + // non-const + { + auto it = intervals.upper_bound(right_open(4, 7)); + assert(it != intervals.end()); + assert(it->lower == 10); + assert(it->upper == 12); + } + + // const + { + auto it = std::as_const(intervals).upper_bound(right_open(4, 7)); + assert(it != intervals.end()); + assert(it->lower == 10); + assert(it->upper == 12); + } + } + + // lower_bound + { + std::set intervals = { + right_open(0, 2), + right_open(3, 5), + right_open(6, 9), + right_open(10, 12), + }; + // non-const + { + auto it = intervals.lower_bound(right_open(1, 4)); + assert(it != intervals.end()); + assert(it->lower == 0); + assert(it->upper == 2); + } + + // const + { + auto it = std::as_const(intervals).lower_bound(right_open(1, 4)); + assert(it != intervals.end()); + assert(it->lower == 0); + assert(it->upper == 2); + } + } + } + + // std::map + { + using X = int; + X const x = 99; + + // upper_bound + { + std::map intervals = { + {right_open(0, 2), x}, + {right_open(3, 5), x}, + {right_open(6, 9), x}, + {right_open(10, 12), x}, + }; + // non-const + { + auto it = intervals.upper_bound(right_open(4, 7)); + assert(it != intervals.end()); + assert(it->first.lower == 10); + assert(it->first.upper == 12); + } + + // const + { + auto it = std::as_const(intervals).upper_bound(right_open(4, 7)); + assert(it != intervals.end()); + assert(it->first.lower == 10); + assert(it->first.upper == 12); + } + } + + // lower_bound + { + std::map intervals = { + {right_open(0, 2), x}, + {right_open(3, 5), x}, + {right_open(6, 9), x}, + {right_open(10, 12), x}, + }; + // non-const + { + auto it = intervals.lower_bound(right_open(1, 4)); + assert(it != intervals.end()); + assert(it->first.lower == 0); + assert(it->first.upper == 2); + } + + // const + { + auto it = std::as_const(intervals).lower_bound(right_open(1, 4)); + assert(it != intervals.end()); + assert(it->first.lower == 0); + assert(it->first.upper == 2); + } + } + } +}