Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conditionally Borrowed Ranges #1877

Merged

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 removed their assignment May 13, 2021
@StephanTLavavej StephanTLavavej self-assigned this May 20, 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