Skip to content

Use _Verify_ranges_do_not_overlap in more places - #5495

Merged
Stephan T. Lavavej (StephanTLavavej) merged 8 commits into
microsoft:mainfrom
AlexGuteniev:do-not-overlap
May 17, 2025
Merged

Use _Verify_ranges_do_not_overlap in more places#5495
Stephan T. Lavavej (StephanTLavavej) merged 8 commits into
microsoft:mainfrom
AlexGuteniev:do-not-overlap

Conversation

@AlexGuteniev

Copy link
Copy Markdown
Contributor

Resolves #5472.

Only used the function in the places where it is a perfect match. So no ranges::swap_ranges with two sentinels, and no std::copy etc with weaker precondition.

I was very surprised to learn that copy_if / remove_copy / remove_copy_if / unique_copy require non-overlap as follows:

The ranges [first, last) and [result, result + (last - first)) do not overlap.

This implies that the destination range may fit the same element number as the source range contains. I thought only fitting the actually returned number of elements is enough.

Overall, now as I've done this, I'm not even sure if it is at all useful.

and you can see where I copied the death test from
@AlexGuteniev

Copy link
Copy Markdown
Contributor Author

libc++ tests fail specifically due to this:

This implies that the destination range may fit the same element number as the source range contains. I thought only fitting the actually returned number of elements is enough.

They have:

      std::array<S, 4> in = {{{4, 2}, {1, 3}, {3, 4}, {3, 5}}};
      std::array<S, 2> out;
      auto ret = std::ranges::copy_if(in.begin(), in.end(), out.begin(), [](int i) { return i == 3; }, &S::val);

https://github.com/llvm/llvm-project/blob/79210feb2993ff9a79ef11f8a7016a527d4fcf22/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/ranges.copy_if.pass.cpp#L160-L162

@AlexGuteniev

Copy link
Copy Markdown
Contributor Author

LLVM-139464 created.

Now that copycats are not enforced to have non-overlap, this looks even less useful.

@AlexGuteniev

Copy link
Copy Markdown
Contributor Author

I was very surprised to learn that copy_if / remove_copy / remove_copy_if / unique_copy require non-overlap as follows:

I think that standard shoould have a weaker precondition here. Just dest not in [first, last)

Comment thread stl/inc/algorithm
Comment thread stl/inc/algorithm Outdated
Comment thread tests/std/tests/GH_005472_do_not_overlap/test.cpp Outdated
@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) moved this from Work In Progress to Initial Review in STL Code Reviews May 15, 2025
@StephanTLavavej

Copy link
Copy Markdown
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej
Stephan T. Lavavej (StephanTLavavej) merged commit c8ae40b into microsoft:main May 17, 2025
@github-project-automation github-project-automation Bot moved this from Merging to Done in STL Code Reviews May 17, 2025
@StephanTLavavej

Copy link
Copy Markdown
Member

⛰️ ❌ 🏔️

@AlexGuteniev
Alex Guteniev (AlexGuteniev) deleted the do-not-overlap branch May 17, 2025 05:02
@AlexGuteniev

Copy link
Copy Markdown
Contributor Author

I was very surprised to learn that copy_if / remove_copy / remove_copy_if / unique_copy require non-overlap as follows:

I think that standard shoould have a weaker precondition here. Just dest not in [first, last)

For the record, this is now LWG-4262

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Something can be improved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

<algorithm>: use _Verify_ranges_do_not_overlap function in more places

2 participants