[ADT] Fix template parameter names of llvm::{upper|lower}_bound

[ilya-biryukov]

Summary

Rename template parameter for a search value from 'ForwardIt' to 'T'.
While here, also use perfect forwarding to pass the value to STL algos.

[ilya-biryukov]

• clang-format

[dblaikie]

Might've made sense to separate the NFC renaming from the perfect forwarding change (& the perfect forwarding change could potentially have some matching test coverage?)

[ilya-biryukov]

In D60510#1467262, @dblaikie wrote:

Might've made sense to separate the NFC renaming from the perfect forwarding change (& the perfect forwarding change could potentially have some matching test coverage?)

I agree, sorry about mashing them in together. That said, I was confident this wouldn't cause any trouble in practice.
What kinds of tests are you thinking about? Checking that llvm::lower_bound works on non-copyable types?

[dblaikie]

In D60510#1468366, @ilya-biryukov wrote:

In D60510#1467262, @dblaikie wrote:

Might've made sense to separate the NFC renaming from the perfect forwarding change (& the perfect forwarding change could potentially have some matching test coverage?)

I agree, sorry about mashing them in together. That said, I was confident this wouldn't cause any trouble in practice.
What kinds of tests are you thinking about? Checking that llvm::lower_bound works on non-copyable types?

Yeah, that'd probably suffice (if you were being extra super robust, and we generally aren't with these thin wrappers, I guess you could test that the parameter can be a temporary and a non-const value where the underlying type or passed in comparator takes by non-const ref (that would test that the forwarding works for things that must be passed by const ref and things that can't be passed by const ref and must be non-const ref))

[ilya-biryukov]

Sure, happy to add some basic tests.
Unfortunately did not get to this today and I'm on vacation until next Tuesday. Will get back to this after I'm back.

llvm/include/llvm/ADT/STLExtras.h

 /// pass begin/end explicitly.
 template <typename R, typename UnaryPredicate>
 auto partition(R &&Range, UnaryPredicate P) -> decltype(adl_begin(Range)) {
   return std::partition(adl_begin(Range), adl_end(Range), P);
 }
 
 /// Provide wrappers to std::lower_bound which take ranges instead of having to
 /// pass begin/end explicitly.
-template <typename R, typename ForwardIt>
-auto lower_bound(R &&Range, ForwardIt I) -> decltype(adl_begin(Range)) {
-  return std::lower_bound(adl_begin(Range), adl_end(Range), I);
+template <typename R, typename T>
+auto lower_bound(R &&Range, T &&Value) -> decltype(adl_begin(Range)) {
+  return std::lower_bound(adl_begin(Range), adl_end(Range),
+                          std::forward<T>(Value));
 }
 
-template <typename R, typename ForwardIt, typename Compare>
-auto lower_bound(R &&Range, ForwardIt I, Compare C)
+template <typename R, typename T, typename Compare>
+auto lower_bound(R &&Range, T &&Value, Compare C)
     -> decltype(adl_begin(Range)) {
-  return std::lower_bound(adl_begin(Range), adl_end(Range), I, C);
+  return std::lower_bound(adl_begin(Range), adl_end(Range),
+                          std::forward<T>(Value), C);
 }
 
 /// Provide wrappers to std::upper_bound which take ranges instead of having to
 /// pass begin/end explicitly.
-template <typename R, typename ForwardIt>
-auto upper_bound(R &&Range, ForwardIt I) -> decltype(adl_begin(Range)) {
-  return std::upper_bound(adl_begin(Range), adl_end(Range), I);
+template <typename R, typename T>
+auto upper_bound(R &&Range, T && Value) -> decltype(adl_begin(Range)) {
+  return std::upper_bound(adl_begin(Range), adl_end(Range),
+                          std::forward<T>(Value));
 }
 
-template <typename R, typename ForwardIt, typename Compare>
-auto upper_bound(R &&Range, ForwardIt I, Compare C)
+template <typename R, typename T, typename Compare>
+auto upper_bound(R &&Range, T && Value, Compare C)
     -> decltype(adl_begin(Range)) {
-  return std::upper_bound(adl_begin(Range), adl_end(Range), I, C);
+  return std::upper_bound(adl_begin(Range), adl_end(Range),
+                          std::forward<T>(Value), C);
 }
 /// Wrapper function around std::equal to detect if all elements
 /// in a container are same.
 template <typename R>
 bool is_splat(R &&Range) {
   size_t range_size = size(Range);
   return range_size != 0 && (range_size == 1 ||
          std::equal(adl_begin(Range) + 1, adl_end(Range), adl_begin(Range)));