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

Johel Ernesto Guerrero Peña via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Thu Aug 18 07:51:55 PDT 2022


JohelEGP added inline comments.


================
Comment at: libcxx/include/__algorithm/lexicographical_compare_three_way.h:46
+    }
+    return __len1 <=> __len2;
+  } else {
----------------
philnik wrote:
> 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.
> In other words IFNDR.

Undefined, by p2.

> What exactly is the difference between "guaranteed" and "requirement"?

The latter is more formal. The former does not imply that it's definitely going to be an integral type. The library gives you license to treat the difference type as an integral type by means of it being anything else being UB. However, I didn't want to confuse the PR submitter by being hand-wavy.

> I'm not sure what you are trying to say here.

Just illustrating an unfortunate example for which you can't call `std::lexicographical_compare_three_way` without it being UB.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131395



More information about the libcxx-commits mailing list