Skip to content

Make some special member functions of shared_mutex, shared_timed_mutex, and shared_lock defaulted#3352

Merged
StephanTLavavej merged 12 commits into
microsoft:mainfrom
frederick-vs-ja:defaulted-shared-mutex
Feb 3, 2023
Merged

Make some special member functions of shared_mutex, shared_timed_mutex, and shared_lock defaulted#3352
StephanTLavavej merged 12 commits into
microsoft:mainfrom
frederick-vs-ja:defaulted-shared-mutex

Conversation

@frederick-vs-ja
Copy link
Copy Markdown
Contributor

@frederick-vs-ja frederick-vs-ja commented Jan 19, 2023

Destructors of shared_mutex and shared_timed_mutex can be implicitly declared. Towards #845.

Destructor of shared_mutex will become trivial after the change. I'm not sure whether this has ABI impact.

@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner January 19, 2023 08:52
CaseyCarter
CaseyCarter reviewed Jan 19, 2023
View reviewed changes
Comment thread stl/inc/shared_mutex
Comment thread stl/inc/shared_mutex Outdated
Comment thread stl/inc/shared_mutex Outdated
Comment thread tests/libcxx/expected_results.txt Outdated
Comment thread tests/libcxx/skipped_tests.txt Outdated
@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label Jan 19, 2023
StephanTLavavej
StephanTLavavej reviewed Jan 24, 2023
View reviewed changes
Comment thread tests/std/tests/Dev11_1150223_shared_mutex/test.cpp Outdated
@StephanTLavavej
Copy link
Copy Markdown
Member

StephanTLavavej commented Jan 24, 2023

Thanks - I've pushed a merge with main to fix trivial adjacent-add conflicts in the libcxx skip lists (curiously, GitHub didn't think there were conflicts, but command-line git did; they might use different auto-merge techniques), plus a typo fix.

I believe that this preserves ABI because the triviality of copy ctors is not affected.

@StephanTLavavej StephanTLavavej self-assigned this Feb 2, 2023
@StephanTLavavej
Copy link
Copy Markdown
Member

StephanTLavavej commented Feb 2, 2023

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

@StephanTLavavej StephanTLavavej merged commit d0c8f14 into microsoft:main Feb 3, 2023
@StephanTLavavej
Copy link
Copy Markdown
Member

StephanTLavavej commented Feb 3, 2023

Thanks for sharing these improvements! 😹 🎉 🚀

@frederick-vs-ja frederick-vs-ja deleted the defaulted-shared-mutex branch February 3, 2023 00: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

+1 more reviewer

@CaseyCarter CaseyCarter CaseyCarter approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@StephanTLavavej StephanTLavavej

Labels

enhancement Something can be improved

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@frederick-vs-ja @StephanTLavavej @CaseyCarter