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

Account for C++17 deferred temporary materialization in invocable_r #1282

Merged
merged 7 commits into from
Sep 23, 2020

Conversation

CaseyCarter
Copy link
Member

...and is_invocable_r_v, as required to be able to initialize std::function<non_movable_type(args...)> (see DevCom-676429). This
fixes DevCom-676429 for Clang, but not MSVC due to a guaranteed copy elision bug VSO-1026729.

...and `is_invocable_r_v`, as required to be able to initialize
`std::function<non_movable_type(args...)>` (see DevCom-676429). This
fixes DevCom-676429 with Clang, but not MSVC due to a guaranteed copy
elision bug VSO-1026729.
@CaseyCarter CaseyCarter added the bug Something isn't working label Sep 9, 2020
@CaseyCarter CaseyCarter requested a review from a team as a code owner September 9, 2020 19:38
@CaseyCarter CaseyCarter added this to Initial Review in Code Reviews via automation Sep 9, 2020
Code Reviews automation moved this from Initial Review to Work In Progress Sep 18, 2020
tests/std/tests/VSO_0105317_expression_sfinae/test.cpp Outdated Show resolved Hide resolved
stl/inc/type_traits Outdated Show resolved Hide resolved
@CaseyCarter CaseyCarter moved this from Work In Progress to Initial Review in Code Reviews Sep 18, 2020
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.

There are ~100 test configs failing; examples:

1: C:\a\1\s\tests\std\tests\Dev11_0535636_functional_overhaul\test.cpp(1647): error C2440: 'initializing': cannot convert from 'test_function::<lambda_8>' to 'std::function<void (std::string &,std::unique_ptr<std::string,std::default_delete<std::string>> &&,std::unique_ptr<int,std::default_delete<int>>)>'
1: C:\a\1\s\tests\std\tests\Dev11_0535636_functional_overhaul\test.cpp(1647): note: No user-defined-conversion operator available that can perform this conversion, or the operator cannot be called
1: C:\a\1\s\tests\std\tests\VSO_0105317_expression_sfinae\test.cpp(31): error C2338: is_invocable_r_v<void, Callable, Args...>
1: C:\a\1\s\tests\std\tests\VSO_0105317_expression_sfinae\test.cpp(38): note: see reference to class template instantiation 'HasInvokeResultT<void,Callable,int>' being compiled
1:         with
1:         [
1:             Callable=FP
1:         ]
1: C:\a\1\s\tests\std\tests\VSO_0105317_expression_sfinae\test.cpp(54): note: see reference to class template instantiation 'HasResultOfT<FP (int)>' being compiled

Code Reviews automation moved this from Initial Review to Work In Progress Sep 18, 2020
@StephanTLavavej StephanTLavavej moved this from Work In Progress to Initial Review in Code Reviews Sep 19, 2020
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.

I'll push a change to fix the duplicated test lines, which look like a copy-paste relic.

tests/std/tests/VSO_0105317_expression_sfinae/test.cpp Outdated Show resolved Hide resolved
stl/inc/type_traits Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej moved this from Initial Review to Final Review in Code Reviews Sep 21, 2020
stl/inc/type_traits Outdated Show resolved Hide resolved
stl/inc/type_traits Outdated Show resolved Hide resolved
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 for salvaging my work! This looks good and your naming is better.

Copy link
Contributor

@mnatsuhara mnatsuhara left a comment

Choose a reason for hiding this comment

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

😵

stl/inc/type_traits Show resolved Hide resolved
@StephanTLavavej StephanTLavavej moved this from Final Review to Ready To Merge in Code Reviews Sep 22, 2020
@StephanTLavavej StephanTLavavej self-assigned this Sep 22, 2020
@StephanTLavavej StephanTLavavej merged commit 7a9998c into microsoft:master Sep 23, 2020
Code Reviews automation moved this from Ready To Merge to Done Sep 23, 2020
@StephanTLavavej
Copy link
Member

Thanks again for fixing this arcane machinery! 🔮

@CaseyCarter CaseyCarter deleted the invocable_r branch September 23, 2020 02:05
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
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants