-
Notifications
You must be signed in to change notification settings - Fork 1.6k
P0439R0 enum class memory_order #124
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
Conversation
Just my 2 cents: it might be a good idea to squash your commits into a single commit. |
They can be squashed at merge time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feature needs to be mentioned in yvals_core.h
in sorted order as being directly controlled by _HAS_CXX20
:
Line 21 in da76ab2
// _HAS_CXX20 directly controls: |
I think that we can manually test and merge this feature, since the test impact should be very small to nonexistent. Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we want testing in place before merging this. I know the change is minor, and I think we could run the testing on your behalf, but I think we at least need a place for the tests to live first.
CC: @imback82 |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
@CaseyCarter #114 needs to merge before that build will succeed. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Pull request contains merge conflicts. |
48ebe1f
to
ae5ecf2
Compare
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine modulo test coverage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finishing an internal build/test before I merge this.
Thanks folks! |
Description
This PR implements #17.
memory_order
becomes anenum class
in cxx20.Checklist:
Working Draft.
_Ugly
as perhttps://eel.is/c++draft/lex.name#3.1 .
by an STL maintainer before CI is online, leave this unchecked for initial
submission). (Internal PR 204684)
members, adding virtual functions, changing whether a type is an aggregate or
trivially copyable, etc.). If unsure, leave this box unchecked and ask a
maintainer for help.