[libcxx-commits] [PATCH] D60592: [libc++] Fix build failure with _LIBCPP_DEBUG=0 when iterators return values instead of references

[Tom Anderson via Phabricator via libcxx-commits]

thomasanderson created this revision.
thomasanderson added a reviewer: EricWF.
Herald added subscribers: jdoerfert, ldionne, christof.
thomasanderson added a comment.

@EricWF ptal.  I think this regressed after https://reviews.llvm.org/rCXX355752


There are many STL algorithms (such as lexicographical_compare) that compare
values pointed to by iterators like so:

      __comp(*it1, *it2);
  	

When building with `_LIBCPP_DEBUG=0`, comparators are wrapped in `__debug_less`
which does some additional validation.  But `__debug_less::operator()` takes
non-const references, so if the type of `*it1` is int, not int&, then the build
will fail.

This change adds a `const&` overload for `operator()` to fix the build.


Repository:
  rCXX libc++

https://reviews.llvm.org/D60592

Files:
  libcxx/include/algorithm
  libcxx/test/libcxx/algorithms/debug_less.pass.cpp


Index: libcxx/test/libcxx/algorithms/debug_less.pass.cpp
===================================================================
--- libcxx/test/libcxx/algorithms/debug_less.pass.cpp
+++ libcxx/test/libcxx/algorithms/debug_less.pass.cpp
@@ -235,10 +235,33 @@
     }
 }
 
+struct ValueIterator {
+    using iterator_category = std::forward_iterator_tag;
+    using value_type = size_t;
+    using difference_type = ptrdiff_t;
+    using reference = size_t&;
+    using pointer = size_t*;
+
+    ValueIterator() = default;
+
+    value_type operator*() { return 0; }
+    ValueIterator& operator++() { return *this; }
+
+    friend bool operator==(ValueIterator, ValueIterator) { return true; }
+    friend bool operator!=(ValueIterator, ValueIterator) { return false; }
+};
+
+void test_value_iterator() {
+    // Ensure no build failures when iterators return values, not references.
+    assert(0 == std::lexicographical_compare(ValueIterator{}, ValueIterator{},
+                                             ValueIterator{}, ValueIterator{}));
+}
+
 int main(int, char**) {
     test_passing();
     test_failing();
     test_upper_and_lower_bound();
     test_non_const_arg_cmp();
+    test_value_iterator();
     return 0;
 }
Index: libcxx/include/algorithm
===================================================================
--- libcxx/include/algorithm
+++ libcxx/include/algorithm
@@ -783,6 +783,15 @@
     _Compare __comp_;
     __debug_less(_Compare& __c) : __comp_(__c) {}
 
+    template <class _Tp, class _Up>
+    bool operator()(const _Tp& __x,  const _Up& __y)
+    {
+        bool __r = __comp_(__x, __y);
+        if (__r)
+            __do_compare_assert(0, __y, __x);
+        return __r;
+    }
+
     template <class _Tp, class _Up>
     bool operator()(_Tp& __x,  _Up& __y)
     {
@@ -792,6 +801,15 @@
         return __r;
     }
 
+    template <class _LHS, class _RHS>
+    inline _LIBCPP_INLINE_VISIBILITY
+    decltype((void)_VSTD::declval<_Compare&>()(
+        _VSTD::declval<const _LHS &>(), _VSTD::declval<const _RHS &>()))
+    __do_compare_assert(int, const _LHS & __l, const _RHS & __r) {
+        _LIBCPP_ASSERT(!__comp_(__l, __r),
+            "Comparator does not induce a strict weak ordering");
+    }
+
     template <class _LHS, class _RHS>
     inline _LIBCPP_INLINE_VISIBILITY
     decltype((void)_VSTD::declval<_Compare&>()(
@@ -801,6 +819,10 @@
             "Comparator does not induce a strict weak ordering");
     }
 
+    template <class _LHS, class _RHS>
+    inline _LIBCPP_INLINE_VISIBILITY
+    void __do_compare_assert(long, const _LHS &, const _RHS &) {}
+
     template <class _LHS, class _RHS>
     inline _LIBCPP_INLINE_VISIBILITY
     void __do_compare_assert(long, _LHS &, _RHS &) {}


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D60592.194788.patch
Type: text/x-patch
Size: 2739 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20190412/17dfc5ad/attachment-0001.bin>