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

Enforce is_clock_v in Clause 32 [thread] headers #1687

Merged
merged 4 commits into from
Mar 2, 2021

Conversation

cpplearner
Copy link
Contributor

Fixes #1680

The relevant requirement in [thread.req.paramname] (along with is_clock_v) was introduced in WG21-P0355. See #12

@cpplearner cpplearner requested a review from a team as a code owner February 23, 2021 09:02
@StephanTLavavej StephanTLavavej added the bug Something isn't working label Feb 23, 2021
@AdamBucior
Copy link
Contributor

Should P2212R2 be noted as implemented in yvals_core.h?

@StephanTLavavej
Copy link
Member

Should P2212R2 be noted as implemented in yvals_core.h?

I considered that paper to be Not Applicable because it doesn't really require implementers to do anything, so I wasn't planning on listing it in yvals_core.h.

Copy link
Member

@StephanTLavavej StephanTLavavej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I verified that all of the class Clock occurrences in the Standard are enforced, and that all of the enforcement here is correct. I'll push a change to remove the one occurrence of unnecessary guards that I found.

stl/inc/condition_variable Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej merged commit 4e38117 into microsoft:main Mar 2, 2021
@StephanTLavavej
Copy link
Member

Thanks for implementing these diagnostics and improving C++20 conformance! 🎉 ✔️ 😸

@frederick-vs-ja
Copy link
Contributor

I think it's conforming to apply the checking to older modes. As only time_point types are involved, and in C++17 it was UB to use time_point<NonClock, Duration> per N4659 [time.point]/1 (the requirement was removed by WG21-P2212R2; perhaps static checking could be OK).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working chrono C++20 chrono
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing is_clock_v enforcement in Clause 32 [thread] headers
5 participants