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

detect forbidden aliasing #1263

Merged
merged 12 commits into from
Aug 12, 2022
Merged

Conversation

Arzaghi
Copy link
Contributor

@Arzaghi Arzaghi commented Sep 4, 2020

Fixes #177

@Arzaghi Arzaghi requested a review from a team as a code owner September 4, 2020 09:44
@CaseyCarter CaseyCarter added the enhancement Something can be improved label Sep 4, 2020
stl/inc/xutility Outdated Show resolved Hide resolved
stl/inc/algorithm Outdated Show resolved Hide resolved
stl/inc/vector Outdated Show resolved Hide resolved
stl/inc/vector Outdated Show resolved Hide resolved
@Arzaghi
Copy link
Contributor Author

Arzaghi commented Sep 7, 2020

As previously mentioned in the original issue(#177), there might be a lot of other candidate functions that need the same enhancement. If you confirm the current pattern, I can apply it to them as well.

stl/inc/algorithm Outdated Show resolved Hide resolved
@StephanTLavavej
Copy link
Member

I resolved a merge conflict in <xutility>, where this PR was moving _Iterators_are_contiguous up, and #1527 recently added _To_pointer to that block of code. Now I just realized that I probably should have built and minimally tested the code before pushing, since _To_pointer might have depended on code in the middle, but we'll see if this works 😹

stl/inc/vector Outdated Show resolved Hide resolved
stl/inc/xutility Outdated Show resolved Hide resolved
stl/inc/xutility Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej removed their assignment Jan 9, 2021
Base automatically changed from master to main January 28, 2021 00:35
@StephanTLavavej StephanTLavavej self-assigned this Apr 28, 2021
This is a squash of some existing work from Arzaghi, plus a bunch of work from nimazzuc

Co-authored-by: Hamid Reza Arzaghi <arzaghi@outlook.com>
@strega-nil-ms
Copy link
Contributor

I've pushed a cleanup and merge commit; this squashes the existing work, since the last "real" commit was from two years ago.

@StephanTLavavej
Copy link
Member

There are numerous test failures that need to be investigated.

@StephanTLavavej StephanTLavavej self-assigned this Aug 10, 2022
stl/inc/xutility Outdated Show resolved Hide resolved
stl/inc/vector Outdated Show resolved Hide resolved
stl/inc/xutility Outdated Show resolved Hide resolved
stl/inc/xutility Outdated Show resolved Hide resolved
stl/inc/xutility Outdated Show resolved Hide resolved
stl/inc/xutility Outdated Show resolved Hide resolved
stl/inc/xutility Outdated Show resolved Hide resolved
@StephanTLavavej
Copy link
Member

Thanks! (And apologies for taking 2 years to review this! 🙀 And thanks @strega-nil-ms for cleaning up the accumulated merge conflicts! 😻) I've pushed some changes, including a major expansion of test coverage:

  • Add preprocessor comment.
  • <xutility> doesn't need to include <cstdio>.
  • Early return within if constexpr.
    • This diff looks horrible until you ignore whitespace.
  • Move call from _Swap_ranges_unchecked to swap_ranges.
  • Drop commented-out code in _Copy_unchecked.
  • Drop unused 4-arg overload.
  • Add positive test coverage.
  • Add negative test coverage.

I explored applying the 4-arg overload to ranges::swap_ranges but that would need more work (non-copyable input iterators failed to compile), so for now only the classic std::swap_ranges is enhanced.

@StephanTLavavej StephanTLavavej removed their assignment Aug 11, 2022
@StephanTLavavej StephanTLavavej self-assigned this Aug 11, 2022
@StephanTLavavej
Copy link
Member

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

@StephanTLavavej StephanTLavavej merged commit fce2d42 into microsoft:main Aug 12, 2022
@StephanTLavavej
Copy link
Member

Thanks again for improving these debugging checks! 🐞 🚀 🎉

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.

<vector>: _ITERATOR_DEBUG_LEVEL could detect forbidden aliasing
6 participants
  翻译: