Skip to content

Conversation

@AdamBucior
Copy link
Contributor

Fixes #1682.

@AdamBucior AdamBucior requested a review from a team as a code owner April 23, 2021 09:41
Copy link
Contributor

@miscco miscco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

Did you verify that we actually test against a borrowed range?

@AdamBucior
Copy link
Contributor Author

Did you verify that we actually test against a borrowed range?

I don't have a deep understanding of the ranges test stuff, but implementing this forced me to fix the common view test, so it seems we do.

@miscco
Copy link
Contributor

miscco commented Apr 23, 2021

Did you verify that we actually test against a borrowed range?

I don't have a deep understanding of the ranges test stuff, but implementing this forced me to fix the common view test, so it seems we do.

As long as we use a span, we should be fine

@miscco
Copy link
Contributor

miscco commented Apr 23, 2021

Did you verify that we actually test against a borrowed range?

I don't have a deep understanding of the ranges test stuff, but implementing this forced me to fix the common view test, so it seems we do.

As long as we use a span, we should be fine

We need to add borrowed range support via a span to take_view and drop_view the other already test it

Something along those lines should do it

    // Validate a non-view borrowed range
    {
        constexpr span s{some_ints};
        STATIC_ASSERT(test_one(s, expected_output));
        test_one(s, expected_output);
    }

    { // Validate a view borrowed range

        constexpr auto v =
            views::iota(0ull, ranges::size(some_ints)) | std::views::transform([](auto i) { return some_ints[i]; });
        STATIC_ASSERT(test_one(v));
        test_one(v);
    }

@StephanTLavavej StephanTLavavej added the LWG Library Working Group issue label Apr 23, 2021
@CaseyCarter CaseyCarter self-assigned this Apr 28, 2021
.... and add coverage of member `data`.
@CaseyCarter CaseyCarter removed their assignment Apr 30, 2021
@StephanTLavavej

This comment has been minimized.

@StephanTLavavej

This comment has been minimized.

@StephanTLavavej StephanTLavavej self-assigned this May 12, 2021
@StephanTLavavej StephanTLavavej merged commit 274faeb into microsoft:main May 21, 2021
@StephanTLavavej
Copy link
Member

Thanks for implementing this LWG issue resolution! 🚀 😻 🎉

@AdamBucior AdamBucior deleted the conditionally-borrowed-ranges branch May 21, 2021 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LWG Library Working Group issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LWG-3494 Allow ranges to be conditionally borrowed

4 participants