microsoft / microsoft/STL

`<ranges>`: `lazy_split_view` should use `_Non_propagating_cache` instead of `_Defaultabox`

Open
#4,273 2 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

bug ranges
Dominant language
C++
Stars
11.1k
Forks
1.7k
Avg merge
4d 15h
Merged PRs (30d)
22

Description

Revealed by libc++ tests std/ranges/range.adaptors/range.lazy.split/ctor.copy_move.pass.cpp and std/ranges/range.adaptors/range.lazy.split/range.lazy.split.inner/iter_swap.pass.cpp.

[range.lazy.split.view] says lazy_split_view<V, Pattern> stores a non-propagating-cache<<iterator_t<V>> if forward_range<V> is false, but MSVC STL's lazy_split_view stores a _Defaultabox:

https://github.com/microsoft/STL/blob/a8888806c6960f1687590ffd4244794c753aa819/stl/inc/ranges#L4612-L4616

This makes lazy_split_view non-copyable when iterator_t<V> is non-copyable.

#include <ranges>
#include <cassert>
using namespace std;

struct MoveOnlyIter {
    using difference_type = int;
    using value_type = int;

    MoveOnlyIter(MoveOnlyIter&&);
    MoveOnlyIter& operator=(MoveOnlyIter&&);

    int operator*() const;
    MoveOnlyIter& operator++();
    void operator++(int);

    bool operator==(int) const;
};

struct TestRange {
    MoveOnlyIter begin();
    int end();
};

void test() {
    TestRange r;
    auto lsv = r | std::views::lazy_split(0);
    auto lsv2 = lsv;  // error on MSVC, should be OK
}

This seems easy to fix, but might require an ABI-breaking change: both _Defaultabox and _Non_propagating_cache have space-efficient specializations, but with different constraints, which means they may have different layout in some cases.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in stl/inc/ranges.html at the lazy_split_view storage referenced by the issue, then compare _Defaultabox with _Non_propagating_cache and their space-efficient specializations. Run std/ranges/range.adaptors/range.lazy.split/ctor.copy_move.pass.cpp and range.lazy.split.inner/iter_swap.pass.cpp; done means the move-only iterator case is copyable and the ABI/layout implications are understood.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
devtools
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.