Proposal: Allow std::from_range and direct std::ranges::to

[Jan Wilken Dörrie]

Hi all,

With Chromium's switch to C++23, I would like to propose moving std::from_range and std::ranges::to from TBD to Allowed.

Proposed Usage:

1. std::from_range: Allows constructing containers directly from a range using the tagged constructor.

// Current C++20-style

std::vector<int> v(input_range.begin(), input_range.end());

// Proposed C++23-style

std::vector<int> v(std::from_range, input_range);

2. std::ranges::to (Direct Call): A powerful utility to convert any range to a container. It is particularly useful for containers like absl::flat_hash_set that do not yet implement the std::from_range constructor. std::ranges::to falls back to the iterator-pair constructor, saving us from manually unpacking iterators.

std::vector<int> vec = GetIds();

// Current: Manual iterator unpacking required

absl::flat_hash_set<int> s(vec.begin(), vec.end());

// Proposed: Clean and explicit conversion

auto s = std::ranges::to<absl::flat_hash_set<int>>(vec);

Clean ups:

• We can remove base::from_range, which is a backport of the C++23 feature.

• We can replace many usages of base::ToVector with the standard std::ranges::to<std::vector<...>>.

Restriction on Adaptor Syntax:

Consistent with the existing ban on range adaptors and pipe syntax in Chromium (due to build-time and readability concerns), I propose keeping the pipe-adaptor version of std::ranges::to banned.

• Allowed: std::ranges::to<std::vector<int>>(input_range)

• Banned: input_range | std::ranges::to<std::vector<int>>()

To enforce this, I plan to update PRESUBMIT.py to allow to in the std::ranges allowlist but add a specific check banning the no-argument version std::ranges::to<...>().

Thoughts?

Thanks,
Jan

[Dominic Battre]

std::ranges::to<std::vector<std::string_view>>(...) would be quite a mouthful. Shall we allow sticking to base::ToVector(...)? I think that the latter wins from a readability perspective.

Best regards,

Dominic

--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To view this discussion visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CALB5Stb-jQgg7YW2cQF_1xRB7GiuYaJn7kdeTVgYT5oaFubH6w%40mail.gmail.com.

[Joe Mason]

Does ‘std::vector<int> v = std::ranges::to(...)‘ work? That's less verbose.

To view this discussion visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAFa1-K0h3AYZcT3nkG%2BVj9cXX9XTBka9myx%2Butojoer8aLe52w%40mail.gmail.com.

[Jan Wilken Dörrie]

> Does std::vector<int> v = std::ranges::to(...) work? That's less verbose.

No, unfortunately that does not work. The result container type needs to be specified explicitly as a template parameter.

> std::ranges::to<std::vector<std::string_view>>(...) would be quite a mouthful. Shall we allow sticking to base::ToVector(...)? I think that the latter wins from a readability perspective.

You don't need to specify the element type, that is std::ranges::to<std::vector>(...) also works. That said, converting to std::vector is common enough of an operation, that it's worth making it as simple and concise as possible. The projection support in base::ToVector is also nice. All of the following would work:

• auto vec = std::ranges::to<std::vector>(input_range);

• std::vector vec(std::from_range, input_range);

• auto vec = base::ToVector(input_range);

Best,
Jan

[Victor Vianna]

> To enforce this, I plan to update PRESUBMIT.py to allow to in the std::ranges allowlist but add a specific check banning the no-argument version std::ranges::to<...>().

Unfortunately this isn't enough to prevent the adaptor use. If the container can be constructed from a range + "something else", that "something else" can be passed as an argument to std::ranges::to. An example is passing a custom compare to a set/map.

https://godbolt.org/z/ecs9rYcoG

Maybe we should allow std::from_range but ban std::ranges::to? That was dcheng@'s suggestion

[Victor Vianna]

Oh, it's also worth mentioning that we have a base::from_range backport that we'd need to remove. I had a draft CL for that crrev.com/c/7380989

[Joe Mason]

`to` is more useful than `from_range`, though, since  absl::flat_hash_map and flat_hash_set don't support from_range.

I think we should write a clang-tidy plugin that gives an error on every use of the overloaded operator| with a range argument. Not as good as a presubmit, but it should apply to every use of it in the ranges API.

I googled "clang c++ pipe operator ast" to see how it was encoded, and the AI result helpfully spit out a partial matcher to start with.

[Joe Mason]

Actually I guess that would go in the chromium-style plugin, which is better than a presubmit.

[Joe Mason]

+tsepez

Since from_range seems uncontroversial, I propose we allow it now, and continue to discuss what to do about `to` in parallel.

[Jan Wilken Dörrie]

>> To enforce this, I plan to update PRESUBMIT.py to allow to in the std::ranges allowlist but add a specific check banning the no-argument version std::ranges::to<...>().
> Unfortunately this isn't enough to prevent the adaptor use. If the container can be constructed from a range + "something else", that "something else" can be passed as an argument to std::ranges::to. An example is passing a custom compare to a set/map.
https://godbolt.org/z/ecs9rYcoG

Thanks for pointing that out. I was aware of this, but figured the PRESUBMIT on the no-arg version should catch the vast majority of accidental usage without any false positives. The bullet-proof fix is indeed the custom clang-plugin to catch all such usages.

> `to` is more useful than `from_range`, though, since absl::flat_hash_map and flat_hash_set don't support from_range.

+1. Wanting to use it with the absl containers was my main motivation to file this proposal. It'd be unfortunate if this usecase wasn't allowed.

> Since from_range seems uncontroversial, I propose we allow it now, and continue to discuss what to do about `to` in parallel.

Agreed. I'm happy to move forward with https://crrev.com/c/7380989.

Best,
Jan

[Victor Vianna]

Sounds good, I sent crrev.com/c/7380989 for review now to allow std::from_range. If someone starts looking into the clang plugin for std::ranges::to, please let us know in this thread to avoid repeated work. 

[Joe Mason]

Ideally the clang plugin would block all uses of | for ranges. (Or block "range adapters", or whatever concrete types are actually banned by https://chromium.googlesource.com/chromium/src/+/main/styleguide/c++/c++-features.md#range-factories-and-range-adaptors-banned.) That way for each new feature we wouldn't need to think about how it interacts with |.

[Jan Wilken Dörrie]

I gave the clang-plugin a shot in https://crrev.com/c/7544914.

[Victor Vianna]

> I gave the clang-plugin a shot in https://crrev.com/c/7544914.

Neat, thanks for working on that!

> `to` is more useful than `from_range`, though, since  absl::flat_hash_map and flat_hash_set don't support from_range.

Added support here.

[Jan Wilken Dörrie]

> Added support here.

Awesome, thanks!

> Neat, thanks for working on that!

No problem :) Considering we'll flag usages of `std::ranges::operator|` soon, are there any outstanding objections to allow `std::ranges::to`? If not, I'll mail out the corresponding CL in a couple of days.

Best,

Jan

[Victor Vianna]

BTW, I started working on a clang-tidy warning to encourage construction via std::from_range rather than iterator pairs (PR, still needs polish, and should maybe be part of modernize-use-ranges instead).

I wondered whether it'd be worth mass-migrating Chromium (CL) but probably not: too much churn to save a few characters.

[Thomas Sepez]

Besides a few chars, vector creation can theoretically be more efficient if it knows the sized range, IIRC.

[Jan Wilken Dörrie]

> Besides a few chars, vector creation can theoretically be more efficient if it knows the sized range, IIRC.

While I would hope that a production quality STL implementation would do that for the iterator constructor as well (as long as they are random access), it is true that the compiler has strictly more information if you pass a whole range. For the `std::from_range` overload the standard explicitly recommends being as efficient as possible: https://eel.is/c++draft/container.requirements#sequence.reqmts-13

> I wondered whether it'd be worth mass-migrating Chromium (CL) but probably not: too much churn to save a few characters.

From a first glance at the line delta (-2714 +3249) it doesn't look worth it, but I think there are two ways you can reduce the number of added lines:

1. Don't explicitly add `#include <ranges>` just for this. While `std::from_range` is defined there, container headers must include it already anyway, since otherwise there'd be no way to define the appropriate constructors. So I'd argue it'd be okay to rely on the transitive #include here.

2. Hard to do automatically, but probably worth it: In many cases you can now avoid creating a temporary, just so that you can call `begin()` and `end()` on it. That is, code like

```

SomeContainer tmp = Foo();

std::vector<int> vec(tmp.begin(), tmp.end());

```

can just be `std::vector<int> vec(std::from_range, Foo());` now, if `tmp` is not used otherwise. 

The potential wins are even larger once we get to the other range inserting member functions, e.g. `vec.append_range(foo);` is quite a bit nicer than `vec.insert(vec.end(), foo.begin(), foo.end())`;

Best,

Jan

[Victor Vianna]

> 1. Don't explicitly add `#include <ranges>` just for this. While `std::from_range` is defined there, container headers must include it already anyway, since otherwise there'd be no way to define the appropriate constructors. So I'd argue it'd be okay to rely on the transitive #include here.

Is that so? I thought libc++ could just forward declare std::from_range_t in <vector> for example (godbolt). 

But seems like it doesn't and maybe that's on purpose? [1][2]

[Victor Vianna]

I also just reproduced a failure due to missing <ranges> in CQ

https://chromium-review.googlesource.com/c/chromium/src/+/7567272

[Jan Wilken Dörrie]

> But seems like it doesn't and maybe that's on purpose? [1][2]

Yeah, that's what I thought. Here are the relevant #includes in our codebase.

> I also just reproduced a failure due to missing <ranges> in CQ

> https://chromium-review.googlesource.com/c/chromium/src/+/7567272

Interesting, so relying on the transitive #include won't work when enabling modules. Apologies for the confusion then, TIL.

[Thomas Sepez]

Hmm. I had an example on godbolt that was shorter for from_range at one point, but maybe our libc++ is smarter...

[Thomas Sepez]

When making vectors from sets, https://godbolt.org/z/j5fo4bhGd seems to show that in the iterator case we make a first loop to determine the size vs just obtaining it directly from the set in the range case.

[Jan Wilken Dörrie]

> When making vectors from sets, https://godbolt.org/z/j5fo4bhGd seems to show that in the iterator case we make a first loop to determine the size vs just obtaining it directly from the set in the range case.

Yeah, that makes sense. In the `std::set` case the iterators are not random access, but only bi-directional. Meaning there is no O(1) way to determine the size, and the implementation needs a loop to find it. Here using `std::from_range` is strictly better.