[libcxx-commits] [PATCH] D131395: [libc++] Implement `lexicographical_compare_three_way`

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/__algorithm/lexicographical_compare_three_way.h:46
+    }
+    return __len1 <=> __len2;
+  } else {
----------------
JohelEGP wrote:
> philnik wrote:
> > avogelsgesang wrote:
> > > mumbleskates wrote:
> > > > philnik wrote:
> > > > > JohelEGP wrote:
> > > > > > huixie90 wrote:
> > > > > > > agree with @philnik that providing benchmark to show the difference.
> > > > > > > 
> > > > > > > question: the standard specifies `return (last1 - first1) <=> (last2 - first2);` but here you are using `size_t` instead of iterator's `difference_type`. so effectively
> > > > > > > `return static_cast<size_t>(last1 - first1) <=> static_cast<size_t>(last2 - first2);`. Does this have an observable difference?
> > > > > > It should if the difference type is an integer-class type.
> > > > > I don't think it has any observable effects, but we shouldn't cast to different types, since that can prohibit some optimizations. 
> > > > i'm not even 100% sure what is allowed to be assumed about `difference_type`; i would definitely avoid using `size_t` here and stick entirely to the iterator's difference type.
> > > Using `difference_type` now, and also only using this optimization if both iterator types have the same, integral `difference_type`
> > `difference_type` is guaranteed to be a signed integral (http://eel.is/c++draft/iterator.traits#2). Otherwise your program if ill-formed. I'd just use `ptrdiff_t` as the type here. No need to not optimize just because the `difference_type`s are different.
> > difference_type is guaranteed to be a signed integral (http://eel.is/c++draft/iterator.traits#2).
> 
> Not guaranteed, but it's a requirement:
> > (4.1) If an algorithm's template parameter is named InputIterator, InputIterator1, or InputIterator2, the template argument shall meet the Cpp17InputIterator requirements ([input.iterators]).
> > -- https://eel.is/c++draft/algorithms.requirements#4.1
> > 1# A class or pointer type X meets the requirements of an input iterator for the value type T if X meets the Cpp17Iterator ([iterator.iterators]) and Cpp17EqualityComparable (Table 29) requirements and the expressions in Table 85 are valid and have the indicated semantics.
> > -- https://eel.is/c++draft/input.iterators#1
> > (2.2) iterator_­traits<X>​::​difference_­type is a signed integer type or void, and
> > -- https://eel.is/c++draft/iterator.iterators#2.2
> 
> > Otherwise your program if ill-formed.
> 
> Not really. It's this:
> > 1# In certain cases (replacement functions, handler functions, operations on types used to instantiate standard library template components), the C++ standard library depends on components supplied by a C++ program. If these components do not meet their requirements, this document places no requirements on the implementation.
> > 2# In particular, the behavior is undefined in the following cases: 
> > [...]
> > (2.3) For types used as template arguments when instantiating a template component, if the operations on the type do not implement the semantics of the applicable Requirements subclause ([allocator.requirements], [container.requirements], [iterator.requirements], [ algorithms.requirements], [numeric.requirements]). Operations on such types can report a failure by throwing an exception unless otherwise specified.
> > [...]
> > -- https://eel.is/c++draft/res.on.functions
> 
> An example for `std::lexicographical_compare_three_way` would be the iterators of an object of type `std::ranges::iota_view<std::intmax_t, std::intmax_t>` (https://eel.is/c++draft/ranges#range.iota.view-1.3).
> > difference_type is guaranteed to be a signed integral (http://eel.is/c++draft/iterator.traits#2).
> 
> Not guaranteed, but it's a requirement:
What exactly is the difference between "guaranteed" and "requirement"? 
> > Otherwise your program if ill-formed.
> 
> Not really. It's this:
> > 1# In certain cases (replacement functions, handler functions, operations on types used to instantiate standard library template components), the C++ standard library depends on components supplied by a C++ program. If these components do not meet their requirements, this document places no requirements on the implementation.
In other words IFNDR.
> An example for `std::lexicographical_compare_three_way` would be the iterators of an object of type `std::ranges::iota_view<std::intmax_t, std::intmax_t>` (https://eel.is/c++draft/ranges#range.iota.view-1.3).
I'm not sure what you are trying to say here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131395/new/

https://reviews.llvm.org/D131395