Proposal: Allow std::ranges::subrange

[Peter Kasting]

TLDR: I propose we allow std::ranges::subrange, with a note that if authors want the code to enforce that the underlying memory is contiguous, they should probably prefer base::span.

Background: Multiple users (on chromium-dev@, Slack #base, etc.) have asked about how to handle pair-of-iterators/iterator+sentinel cases. While base::span is a good default choice, some use cases don't care whether the underlying data happens to be contiguous (e.g. for data stored in a base::flat_map, that might just as well be in a std::map someday) or actually have non-contiguous data. In these cases we offer no ergonomic choice today.

The ideal long-term solution here is unclear. Memory safety folks note that pair-of-iterators datatypes risk UNSAFE_BUFFERS issues. Dana filed an llvm bug seeking clarity on unsafe buffer annotations in libc++.

In the meantime, I think subrange is an OK view to carve out support for. It doesn't make things worse than status quo -- it's a slim wrapper around a pair of iterators, so it doesn't have lazy eval and some of the other issues with STL view types. It doesn't require us to support chaining via pipes. If we want to replace it with a base::subrange that has more qualifiers on it, our API would probably look similar, and having people using the STL type directly will encapsulate the callsites so we can understand the scope of any such transform.

To me the biggest question is whether we also guide people to prefer span where feasible. I think the answer should be "yes, but base it not on 'does it compile' but on whether you want to enforce that the underlying data remains contiguous."  Using span in other cases poses a refactoring hazard in the future, while giving no guidance at all probably sacrifices some level of safety.

PK

[danakj]

On Mon, Sep 23, 2024 at 1:48 PM Peter Kasting <pkas...@chromium.org> wrote:

TLDR: I propose we allow std::ranges::subrange, with a note that if authors want the code to enforce that the underlying memory is contiguous, they should probably prefer base::span.

Background: Multiple users (on chromium-dev@, Slack #base, etc.) have asked about how to handle pair-of-iterators/iterator+sentinel cases. While base::span is a good default choice, some use cases don't care whether the underlying data happens to be contiguous (e.g. for data stored in a base::flat_map, that might just as well be in a std::map someday) or actually have non-contiguous data. In these cases we offer no ergonomic choice today.

The ideal long-term solution here is unclear. Memory safety folks note that pair-of-iterators datatypes risk UNSAFE_BUFFERS issues. Dana filed an llvm bug seeking clarity on unsafe buffer annotations in libc++.

In the meantime, I think subrange is an OK view to carve out support for. It doesn't make things worse than status quo -- it's a slim wrapper around a pair of iterators, so it doesn't have lazy eval and some of the other issues with STL view types. It doesn't require us to support chaining via pipes. If we want to replace it with a base::subrange that has more qualifiers on it, our API would probably look similar, and having people using the STL type directly will encapsulate the callsites so we can understand the scope of any such transform.

+1, we need a vocabulary for working with a range (or subrange) of a non-contiguous container. We explicitly don't want to require code to work with iterator pairs when we don't have to.

 

To me the biggest question is whether we also guide people to prefer span where feasible. I think the answer should be "yes, but base it not on 'does it compile' but on whether you want to enforce that the underlying data remains contiguous."  Using span in other cases poses a refactoring hazard in the future, while giving no guidance at all probably sacrifices some level of safety.

span is richer and better suited when working with contiguous data, such as an array or vector. We should definitely advise folks to continue down that path. flat_map is contiguous as an implementation detail so it's less suitable there.

 

PK

--
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 on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAAHOzFCwO0kP%3DmDVz%2BdkScnQ_V1iNs%2BzaDML4wPLpAMTuPuVvQ%40mail.gmail.com.

[Daniel Cheng]

+1 for allowing subrange.

As for span vs subrange, I would encourage people to use the "right" thing. While span can technically adapt a flat_map, if the abstraction the code using a span wants is "a sorted range", then a subrange is almost certainly a better fit. Otherwise, code at a distance using span will randomly break if we do a mass migration of, say, flat_map to btree_map.

Daniel

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAHtyhaR-rGPC2efbmNT-81-LmCxoc1mLevPxCUSUPojRZMSVcA%40mail.gmail.com.

[Peter Kasting]

Sounds like there's consensus to allow this. I will take a stab at updating the styleguide "soon" unless someone else does first.

PK

[Peter Kasting]

(Now from the right account!)

I've now uploaded this at https://chromium-review.googlesource.com/c/chromium/src/+/5941796, so this should be allowed soon.

PK