Is std::views::keys guaranteed to work correctly with any pair/tuple type?

user_17841694 0
Viewed 114

Code is pasted here & https://www.godbolt.org/z/qszqYsT4o

I am attempting to provide boost::adaptors::indexed functionality that is compatible with c++20 views. My primary use case is for std::vector, but I would definitely prefer a generic solution. I was shocked to discover that std::views::keys did not work as expected to extract the first element correctly.

For GCC 10.3, the output of rng1 is garbage and rng2 is as expected. GCC 10.4+ works fine. The latest version of clang fails to compile the code.

Questions:

  1. Is std::views::keys guaranteed to support any pair/tuple type, or is my code UB?
  2. Given clang 14.0.0 fails to compile, is my code legal C++?
  3. Is there a better way to achieve this functionality in c++20? I was looking at std::span for a bit, but couldn't seem to make it work naturally. Note: I would have gladly used std::views::zip with std::views::iota if it were available.
#include <vector>
#include <ranges>
#include <iostream>

template <typename T>
using IndexValuePair = std::pair<std::size_t, std::reference_wrapper<T>>;

template <typename T, typename Allocator>
auto make_indexed_view(std::vector<T, Allocator>& vec)
{
    auto fn = [&vec](T& val) {
        return IndexValuePair<T>{static_cast<std::size_t>(&val - vec.data()), val};
    };
    return std::views::all(vec) | std::views::transform(fn);
}

template <typename T, typename Allocator>
auto make_indexed_view(const std::vector<T, Allocator>& vec)
{
    auto fn = [&vec](const T& val) {
        return IndexValuePair<const T>{static_cast<std::size_t>(&val - vec.data()), val};
    };
    return std::views::all(vec) | std::views::transform(fn);
}

struct GetFirstSafely {
    template <typename T1, typename T2>
    const T1& operator()(const std::pair<T1, T2>& p) const { return std::get<0>(p); }

    template <typename T1, typename T2>
    T1 operator()(std::pair<T1, T2>&& p) const { return std::get<0>(std::move(p)); }
};

auto get_first = [](auto&& p) -> decltype(auto) { return GetFirstSafely{}(std::forward<decltype(p)>(p)); };

int main()
{
    const std::vector<int> v{10, 20, 30};
    auto fn = [](const auto& val) { return val.second >= 20; };
    auto rng1 = make_indexed_view(v) | std::views::filter(fn) | std::views::keys;
    auto rng2 = make_indexed_view(v) | std::views::filter(fn) | std::views::transform(get_first);
    for (auto&& elem : rng1)
        std::cout << elem << '\n';
    for (auto&& elem : rng2)
        std::cout << elem << '\n';
    return 0;
}
std-ranges c++20 c++
3 Answers

Is std::views::keys guaranteed to support any pair/tuple type, or is my code UB?

views::keys is guaranteed to work with pair/tuple in C++20, and is guaranteed to work with tuple-like objects in C++23 thanks to P2165.

Given clang 14.0.0 fails to compile, is my code legal C++?

Your code is well-formed.

However, it is not necessary to use reference_wrapper, an simple pair<size_t, T&> should be enough, and there is no need to use std::views::all(vec), vec | views::transform(fn) will automatically convert vec to view.

Is there a better way to achieve this functionality in c++20?

In C++20, I'm afraid not. In C++23 you can compose views::iota and views::zip to do this, e.g.

const std::vector<int> v{10, 20, 30};
for (const auto& [index, value] : views::zip(views::iota(0uz, v.size()), v))
  std::cout << index << " " << value << "\n";

In C++26 you can use views::enumerate (if it is adopted)

const std::vector<int> v{10, 20, 30};
for (const auto & e : views::enumerate(v))
  std::cout << e.index << " " << e.value << "\n";
user_116387180

This was LWG 3502. Your code fails on gcc 10.3 because of that particular issue, but it has been resolved and your code works fine on gcc 10.4 (or 11.1, etc.).

The example in the issue there should look familiar:

std::vector<int> vec = {42};
auto r = vec | std::views::transform([](auto c) { return std::make_tuple(c, c); })
             | std::views::keys;

It failed because operator* for elements_view was specified as:

constexpr decltype(auto) operator*() const { return get<N>(*current_); }

And when the Nth element is a prvalue (as it is in that example and your example), this is an immediately-dangling rvalue reference. The resolution in the issue ensures that operator* in these cases returns a prvalue, so everything works fine.

Note that there's no reason to write:

return std::views::all(vec) | std::views::transform(fn);

The range adaptors already do that for you. You can just write:

return vec | std::views::transform(fn);
user_20690640

It is possible to use std::span for make_indexed_view to make it more generic. Of course, modifying the structure of the underlying sequence (e.g. calling std::vector::push_back) is undefined behavior.

template <typename T, std::size_t Extent>
auto make_indexed_view(const std::span<T, Extent>& span)
{
    auto fn = [span](T& val) {
        return IndexValuePair<T>{static_cast<std::size_t>(&val - span.data()), val};
    };
    return span | std::views::transform(fn);    
}

int main()
{
    constexpr int data[] = {10, 20, 30};
    auto fn = [](const auto& elem) { return elem.second >= 20; };
    auto rng = make_indexed_view(std::span{data}) | std::views::filter(fn) | std::views::transform(get_first);
    for (auto&& elem : rng) {
        std::cout << elem << '\n';
    }
    return 0;
}
user_178416940
Related