[libcxx-commits] [libcxx] [libc++] Optimize ranges::move{, _backward} for vector<bool>::iterator (PR #121109)

[Peng Liu via libcxx-commits]

================
@@ -0,0 +1,70 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+
+#include <algorithm>
+#include <benchmark/benchmark.h>
+#include <ranges>
+#include <vector>
+
+template <bool aligned>
+void bm_ranges_move_vb(benchmark::State& state) {
+  auto n = state.range();
+  std::vector<bool> v1(n, true);
+  std::vector<bool> v2(n, false);
+  benchmark::DoNotOptimize(v1);
+  benchmark::DoNotOptimize(v2);
+  std::vector<bool>* in  = &v1;
+  std::vector<bool>* out = &v2;
+  for (auto _ : state) {
+    if constexpr (aligned) {
+      benchmark::DoNotOptimize(std::ranges::move(*in, std::ranges::begin(*out)));
+    } else {
+      benchmark::DoNotOptimize(std::ranges::move(*in | std::views::drop(4), std::ranges::begin(*out)));
+    }
----------------
winner245 wrote:

I agree with you that using `subrange` would be more intuitive than using `drop_view/take_view`. The reason I used `drop_view/take_view` is that it makes my tests for range algorithms almost identical for aligned and unaligned cases using the pipelined call. However, I do agree that clarity is more important here, and subrange does make it more obvious that we are guaranteed to operate with `vector<bool>::iterator`s. Therefore, I will update all the tests and benchmarks to use `subrange`.

I would also like to justify that my previous approach of using `drop_view/take_view` was correct, just to confirm my understanding. I believe `drop_view` (as well as `take_view`) of `vector<bool>` guarantees us to get back to `vector<bool>::iterator` when we call `begin()` on the view. This is not due to the iterator unwrapping performed within `std::move` (e.g., `std::__unwrap_iter`). It is guaranteed by the views themselves. 

According to [[range.drop.view]](https://eel.is/c++draft/range.drop.view), `drop_view<V>::begin()` returns `ranges​::​next(ranges​::​begin(base_), count_, ranges​::​end(base_))`, where `base_` is the underlying (adapted) view of type V (i.e., a view of `vector<bool>`). This implies that `drop_view<V>::begin()` has the same return type as the type of `ranges​::​begin(base_)`, and hence of `base_.begin()`, which is `vector<bool>::iterator`. 

The same thing applies to `take_view<V>::begin()`. For `take_view`, when the view is `sized_range` (true for `vector<bool>`), the return type of `take_view<V>::begin()` is the same as `ranges​::​begin(base_)`. 

I have a check for this: https://godbolt.org/z/xj7fMaeWf

```cpp
#include <vector>
#include <ranges>

constexpr bool test() {
  std::vector<bool> a{true, false, true};
  auto view = a | std::views::drop(1);
  auto begin = view.begin();
  return std::is_same_v<decltype(begin), std::vector<bool>::iterator>;
}

int main() {
  static_assert(test());
}
```



https://github.com/llvm/llvm-project/pull/121109