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

constexpr mutex #2309

Closed
wants to merge 21 commits into from
Closed

constexpr mutex #2309

wants to merge 21 commits into from

Conversation

AlexGuteniev
Copy link
Contributor

Fixes #2285

@AlexGuteniev
Copy link
Contributor Author

Apparently the tests fail/hang because of #2311

@AlexGuteniev
Copy link
Contributor Author

AlexGuteniev commented Oct 31, 2021

Closing, details in the issue #2285 (comment)

Edit: reopened, but still looks risky, want to ask if it is ok to have this approach

@AlexGuteniev AlexGuteniev deleted the mutex branch October 31, 2021 09:27
@AlexGuteniev AlexGuteniev restored the mutex branch October 31, 2021 09:52
@AlexGuteniev AlexGuteniev reopened this Oct 31, 2021
@AlexGuteniev AlexGuteniev marked this pull request as ready for review October 31, 2021 12:54
@AlexGuteniev AlexGuteniev requested a review from a team as a code owner October 31, 2021 12:54
stl/inc/mutex Outdated Show resolved Hide resolved
@CaseyCarter CaseyCarter added the bug Something isn't working label Nov 1, 2021
@AlexGuteniev AlexGuteniev marked this pull request as draft November 1, 2021 21:19
@AlexGuteniev

This comment has been minimized.

@AlexGuteniev

This comment has been minimized.

@AlexGuteniev AlexGuteniev marked this pull request as ready for review November 2, 2021 18:15
AlexGuteniev added a commit to AlexGuteniev/STL that referenced this pull request Nov 2, 2021
Resolves microsoft#2286

Wanted to combine with microsoft#2309, but apparently constexpr mutex
will have long way to go if ever succeeds
@AlexGuteniev AlexGuteniev mentioned this pull request Nov 2, 2021
@AlexGuteniev
Copy link
Contributor Author

Blocked by DevCom-1570141

stl/inc/mutex Outdated
_Mutex_base(int _Flags = 0) noexcept {
#if _HAS_CXX20
if (_STD is_constant_evaluated()) {
::new (static_cast<void*>(&_Mtx_storage)) _Stl_critical_section_constexpr{};
Copy link
Contributor

@CaseyCarter CaseyCarter Nov 2, 2021

Choose a reason for hiding this comment

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

This new-expression isn't valid in a constant expression since it doesn't select a replaceable global allocation function ([expr.const]/5.19). We could use construct_at, but it will need storage for an actual _Stl_critical_section_constexpr to target - presumably in a union, which will make mutex not standard-layout.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This new-expression isn't valid in a constant expression since

What if I use custom form of placement new, that would be constexpr?

which (I think) will make mutex not standard-layout

Right, we can't do union here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed, seem to compile

Copy link
Contributor

Choose a reason for hiding this comment

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

This new-expression isn't valid in a constant expression since

What if I use custom form of placement new, that would be constexpr?

Either it wouldn't really allocate storage ("select a replaceable global allocation function") or it wouldn't deallocate the storage within the evaluation of the constant expression (also [expr.const]/5.19).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got your point. But what if the non-standard code still compiles?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I put construct_at there, but with a static_cast. Is this still a crime?

Copy link
Contributor

Choose a reason for hiding this comment

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

That violates [expr.const]/5.15 since it involves "a conversion from type cv void* to a pointer-to-object type".

@AlexGuteniev
Copy link
Contributor Author

Ok, I give up again.
I cannot find a compliant way to put non-standard-layout class in a standard layout class.
A standard-layout class (with vtable made as struct) is too dangerous for @StephanTLavavej.

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

Successfully merging this pull request may close these issues.

<mutex> : std::mutex::mutex is not constexpr
3 participants