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

Implement P2655R3 common_reference_t Of reference_wrapper Should Be A Reference Type #3513

Merged
merged 6 commits into from
Mar 3, 2023

Conversation

frederick-vs-ja
Copy link
Contributor

Fixes #3444.

I think the changes should be applied to C++20 mode as this documentation said P2655 is possibly a DR.

Also

@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner February 28, 2023 17:47
Copy link
Contributor

@strega-nil-ms strega-nil-ms left a comment

Choose a reason for hiding this comment

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

Minor questions, otherwise looks good!

stl/inc/type_traits Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej added the cxx23 C++23 feature label Feb 28, 2023
stl/inc/yvals_core.h Outdated Show resolved Hide resolved
stl/inc/type_traits Outdated Show resolved Hide resolved
stl/inc/type_traits Outdated Show resolved Hide resolved
@StephanTLavavej
Copy link
Member

Thanks, this looks solid, and I agree that implementing it in C++20 mode is reasonable! 😻 (This aligns with our usual rationale - if a paper deeply modifies an existing feature in a way that doesn't introduce new names, isn't likely to be source-breaking, is generally experienced as a strict improvement, and would be obnoxious to conditionally maintain, then implementing it retroactively is desirable. This is very much like the enable_shared_from_this overhaul.)

I've pushed some nitpicky cosmetic changes, nothing functional. FYI @strega-nil-ms as you already approved.

@StephanTLavavej StephanTLavavej self-assigned this Mar 2, 2023
@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 ec7cc0f into microsoft:main Mar 3, 2023
@StephanTLavavej
Copy link
Member

Thanks for enhancing some of the STL's most intricate template machinery! ⚙️ ✨ 🚀

@frederick-vs-ja frederick-vs-ja deleted the p2655r3 branch March 3, 2023 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cxx23 C++23 feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

P2655R3 common_reference_t Of reference_wrapper Should Be A Reference Type
3 participants