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.

Sign up for GitHub

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

Jump to bottom

<atomic>: Add member difference_type to atomic<void*> and its friends #4689

Merged
merged 4 commits into from
May 30, 2024
Merged

<atomic>: Add member difference_type to atomic<void*> and its friends #4689

merged 4 commits into from
May 30, 2024

Conversation

frederick-vs-ja
Copy link
Contributor

Fixes #4688.

It seems intentional that atomic(_ref)<T*> are SFINAE-friendly on atomic pointer arithmetic. So this PR adds another mediate base class to avoid breaking the SFINAE-friendliness.

Also updates the comments in Dev11_0863628_atomic_compare_exchange/test.cpp to cite WG21-N4981.

@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner May 23, 2024 17:43
@frederick-vs-ja
Add member difference_type to atomic<void*> and its friends
1cb0715 
It seems intentional that `atomic(_ref)<T*>` are SFINAE-friendly on
atomic pointer arithmetic. So this PR adds another mediate base class
to avoid breaking the SFINAE-friendliness.

Also updates the comments in
`Dev11_0863628_atomic_compare_exchange/test.cpp` to cite WG21-N4981.
@cpplearner
Copy link
Contributor

It seems intentional that atomic(_ref)<T*> are SFINAE-friendly on atomic pointer arithmetic. So this PR adds another mediate base class to avoid breaking the SFINAE-friendliness.

I don't think that matches the standard, which uses "Mandates" ([atomics.types.pointer]/5, [atomics.ref.pointer]/3).

@frederick-vs-ja
Copy link
Contributor Author

frederick-vs-ja commented May 24, 2024 •
edited
Loading

It seems intentional that atomic(_ref)<T*> are SFINAE-friendly on atomic pointer arithmetic. So this PR adds another mediate base class to avoid breaking the SFINAE-friendliness.

I don't think that matches the standard, which uses "Mandates" ([atomics.types.pointer]/5, [atomics.ref.pointer]/3).

Mandates doesn't mean the ill-formedness shall be SFINAE-unfriendly, although we ususally use static_asserts or straightforward hard errors for it.

Previously, #4014 made Mandates for forward_like SFINAE-friendly.

Edit: see also #3013 (comment).

@StephanTLavavej StephanTLavavej added the bug Something isn't working label May 28, 2024
@StephanTLavavej StephanTLavavej self-assigned this May 28, 2024
@StephanTLavavej
Copy link
Member

Thanks for the quick fix and comprehensive test coverage, this is great! 😻

@StephanTLavavej StephanTLavavej removed their assignment May 29, 2024
@StephanTLavavej StephanTLavavej mentioned this pull request May 29, 2024
Open
@StephanTLavavej StephanTLavavej self-assigned this May 29, 2024
@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 d56c6db into microsoft:main May 30, 2024
39 checks passed
@StephanTLavavej
Copy link
Member

Thanks for making a difference! 😹 🤪 💚

@frederick-vs-ja frederick-vs-ja deleted the atomic-ptr-diff branch May 30, 2024 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

Assignees

@StephanTLavavej StephanTLavavej

Labels
bug Something isn't working
Projects
STL Code Reviews
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

<atomic>: atomic_ref<void*> and atomic<void*> lack difference_type
3 participants
@frederick-vs-ja @cpplearner @StephanTLavavej