[libcxx] r345434 - Fix PR39458 _LIBCPP_DEBUG breaks heterogeneous compare.

[Eric Fiselier]

Author: ericwf
Date: Fri Oct 26 15:54:46 2018
New Revision: 345434

URL: http://llvm.org/viewvc/llvm-project?rev=345434&view=rev
Log:
Fix PR39458 _LIBCPP_DEBUG breaks heterogeneous compare.

The types/comparators passed to std::upper_bound and std::lower_bound
are not required to provided to provide an operator</comp(...) which
accepts the arguments in reverse order. Nor are the ranges required
to have a strict weak ordering.

However, in debug mode we attempted to check the result of a comparison
with the arguments reversed, which may not compiler.

This patch removes the use of the debug comparator for upper_bound
and lower_bound.

equal_range et al still use debug comparators when they call
__upper_bound and __lower_bound.

See llvm.org/PR39458

Modified:
    libcxx/trunk/include/algorithm
    libcxx/trunk/test/libcxx/algorithms/debug_less.pass.cpp

Modified: libcxx/trunk/include/algorithm
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/algorithm?rev=345434&r1=345433&r2=345434&view=diff
==============================================================================
--- libcxx/trunk/include/algorithm (original)
+++ libcxx/trunk/include/algorithm Fri Oct 26 15:54:46 2018
@@ -4088,14 +4088,8 @@ inline _LIBCPP_INLINE_VISIBILITY _LIBCPP
 _ForwardIterator
 lower_bound(_ForwardIterator __first, _ForwardIterator __last, const _Tp& __value_, _Compare __comp)
 {
-#ifdef _LIBCPP_DEBUG
-    typedef typename add_lvalue_reference<__debug_less<_Compare> >::type _Comp_ref;
-    __debug_less<_Compare> __c(__comp);
-    return __lower_bound<_Comp_ref>(__first, __last, __value_, __c);
-#else  // _LIBCPP_DEBUG
     typedef typename add_lvalue_reference<_Compare>::type _Comp_ref;
     return __lower_bound<_Comp_ref>(__first, __last, __value_, __comp);
-#endif  // _LIBCPP_DEBUG
 }
 
 template <class _ForwardIterator, class _Tp>
@@ -4136,14 +4130,8 @@ inline _LIBCPP_INLINE_VISIBILITY _LIBCPP
 _ForwardIterator
 upper_bound(_ForwardIterator __first, _ForwardIterator __last, const _Tp& __value_, _Compare __comp)
 {
-#ifdef _LIBCPP_DEBUG
-    typedef typename add_lvalue_reference<__debug_less<_Compare> >::type _Comp_ref;
-    __debug_less<_Compare> __c(__comp);
-    return __upper_bound<_Comp_ref>(__first, __last, __value_, __c);
-#else  // _LIBCPP_DEBUG
     typedef typename add_lvalue_reference<_Compare>::type _Comp_ref;
     return __upper_bound<_Comp_ref>(__first, __last, __value_, __comp);
-#endif  // _LIBCPP_DEBUG
 }
 
 template <class _ForwardIterator, class _Tp>

Modified: libcxx/trunk/test/libcxx/algorithms/debug_less.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/algorithms/debug_less.pass.cpp?rev=345434&r1=345433&r2=345434&view=diff
==============================================================================
--- libcxx/trunk/test/libcxx/algorithms/debug_less.pass.cpp (original)
+++ libcxx/trunk/test/libcxx/algorithms/debug_less.pass.cpp Fri Oct 26 15:54:46 2018
@@ -161,7 +161,58 @@ void test_failing() {
     }
 }
 
+template <int>
+struct Tag {
+  explicit Tag(int v) : value(v) {}
+  int value;
+};
+
+template <class = void>
+struct FooImp {
+  explicit FooImp(int x) : x_(x) {}
+  int x_;
+};
+
+template <class T>
+inline bool operator<(FooImp<T> const& x, Tag<0> y) {
+    return x.x_ < y.value;
+}
+
+template <class T>
+inline bool operator<(Tag<0>, FooImp<T> const&) {
+    static_assert(sizeof(FooImp<T>) != sizeof(FooImp<T>), "should not be instantiated");
+}
+
+template <class T>
+inline bool operator<(Tag<1> x, FooImp<T> const& y) {
+    return x.value < y.x_;
+}
+
+template <class T>
+inline bool operator<(FooImp<T> const&, Tag<1>) {
+    static_assert(sizeof(FooImp<T>) != sizeof(FooImp<T>), "should not be instantiated");
+}
+
+typedef FooImp<> Foo;
+
+// Test that we don't attempt to call the comparator with the arguments reversed
+// for upper_bound and lower_bound since the comparator or type is not required
+// to support it, nor does it require the range to have a strict weak ordering.
+// See llvm.org/PR39458
+void test_upper_and_lower_bound() {
+    Foo table[] = {Foo(1), Foo(2), Foo(3), Foo(4), Foo(5)};
+    {
+        Foo* iter = std::lower_bound(std::begin(table), std::end(table), Tag<0>(3));
+        assert(iter == (table + 2));
+    }
+    {
+        Foo* iter = std::upper_bound(std::begin(table), std::end(table), Tag<1>(3));
+        assert(iter == (table + 3));
+    }
+}
+
 int main() {
     test_passing();
     test_failing();
+    test_upper_and_lower_bound();
 }