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 LWG-3655: The INVOKE operation and union types #3495

Merged
merged 4 commits into from
Feb 26, 2023

Conversation

fsb4000
Copy link
Contributor

@fsb4000 fsb4000 commented Feb 23, 2023

Fixes #3424

@fsb4000 fsb4000 requested a review from a team as a code owner February 23, 2023 13:35
@github-actions github-actions bot added this to Initial Review in Code Reviews Feb 23, 2023
@CaseyCarter CaseyCarter added the LWG Library Working Group issue label Feb 23, 2023
@CaseyCarter CaseyCarter moved this from Initial Review to Final Review in Code Reviews Feb 23, 2023
tests/std/tests/P2136R3_invoke_r/test.cpp Outdated Show resolved Hide resolved
@@ -109,6 +109,13 @@ constexpr bool test_invoke_r() {
return true;
}

// LWG-3655: The INVOKE operation and union types
Copy link
Member

Choose a reason for hiding this comment

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

No change requested: this LWG issue resolution is being implemented unconditionally (as usual), but the test being extended here is C++23:

RUNALL_INCLUDE ..\usual_latest_matrix.lst

This means that the product code is active for C++14/17/20, but is not being tested in those modes. We have tests that are built down to C++14 mode (e.g. Dev11_0535636_functional_overhaul is where INVOKE is mostly tested).

However, I don't think this coverage needs to be moved - even with my paranoia about compiler bugs, I find it hard to imagine a compiler bug that would affect this code that would appear only in C++14/17/20 mode. Also, it's slightly annoying to test anything here in C++14 mode as all that's available there is the older/deprecated result_of. So I'm simply noting this for future consideration.

tests/std/tests/P2136R3_invoke_r/test.cpp Outdated Show resolved Hide resolved
@StephanTLavavej
Copy link
Member

Thanks! I've pushed a nitpicky rename and an expansion of the test coverage. FYI @CaseyCarter.

@StephanTLavavej StephanTLavavej moved this from Final Review to Ready To Merge in Code Reviews Feb 23, 2023
@StephanTLavavej StephanTLavavej self-assigned this Feb 24, 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 4ba94ca into microsoft:main Feb 26, 2023
Code Reviews automation moved this from Ready To Merge to Done Feb 26, 2023
@StephanTLavavej
Copy link
Member

Thanks for extending INVOKE to handle even more types! 📈 🎉 💯

@fsb4000 fsb4000 deleted the fix3424 branch February 26, 2023 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LWG Library Working Group issue
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

LWG-3655 The INVOKE operation and union types
4 participants