Page MenuHomePhabricator
Differential D93815

[libc++] [P1065] Constexpr invoke, reference_wrapper, mem_fn, not_fn.
ClosedPublic
Actions

Authored by Quuxplusone on Dec 25 2020, 1:10 PM.

Details

Reviewers
ldionne
zoecarver
Group Reviewers
Restricted Project
Commits
rG7b00e9fae385: [libc++] [P1065] Constexpr invoke, reference_wrapper, mem_fn, not_fn…
Summary

I believe this is a complete implementation of P1065R2 "Constexpr INVOKE" (Tomasz Kamiński, Barry Revzin), which was adopted for C++20.
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1065r2.html

This could use more constexpr tests for std::reference_wrapper<T>, but the existing tests are extremely constexpr-unfriendly and so I don't want to get into that rabbit-hole today.

I plan to push the refactoring of ForwardingCallObject as a separate preliminary commit:

[libc++] Constexpr-proof some machinery in not_fn.pass.cpp. NFCI.

Diff Detail

Event Timeline

Quuxplusone requested review of this revision.Dec 25 2020, 1:10 PM
Quuxplusone created this revision.
Herald added a reviewer: Restricted Project. · View Herald TranscriptDec 25 2020, 1:10 PM
Herald added a subscriber: libcxx-commits. · View Herald Transcript
Quuxplusone updated this revision to Diff 313739.Dec 25 2020, 6:11 PM

Complete the implementation of __cpp_lib_constexpr_functional and set that feature macro. This involves constexprifying std::default_searcher from P1032 "Misc constexpr bits". There's more "misc" stuff in P1032, which I have not attempted to do in this PR because it doesn't affect the __cpp_lib_constexpr_functional feature macro.

https://isocpp.org/std/standing-documents/sd-6-sg10-feature-test-recommendations#library-feature-test-macros

Herald added a subscriber: arichardson. · View Herald TranscriptDec 25 2020, 6:11 PM
Harbormaster completed remote builds in B83508: Diff 313739.Dec 25 2020, 6:11 PM
Harbormaster completed remote builds in B83503: Diff 313734.Dec 25 2020, 7:15 PM
zoecarver accepted this revision.Dec 26 2020, 7:32 PM
zoecarver added a subscriber: zoecarver.

This LGTM. Thanks. Looking forward to using this in my bind_front patch ;)

libcxx/test/std/utilities/function.objects/func.invoke/invoke_constexpr.pass.cpp
270

I assume it didn't make sense to merge these with the existing tests because of functions like foo which would only work in one mode or the other?

Is there any test coverage here that isn't in the non-constexpr version? If so, would it make sense to run these tests outside of a constexpr context as well (it very well might not)?

libcxx/test/std/utilities/function.objects/func.not_fn/not_fn.pass.cpp
548

This is just because std::string isn't constexpr?

Quuxplusone marked an inline comment as done.Dec 26 2020, 8:37 PM
Quuxplusone added inline comments.
libcxx/test/std/utilities/function.objects/func.invoke/invoke_constexpr.pass.cpp
270

A mix of that reason, and the reason that I wanted to make sure constexprifying things didn't somehow break the non-constexpr codepath. One thing we haven't been worrying about much with these patches, and which I suddenly decided to worry about for this patch (why? I don't know) is that when we mark all the helper/component functions like foo with TEST_CONSTEXPR_CXX20, we are losing test coverage of how the thing works with non-constexpr functions. For invoke, that suddenly seemed important to test.

There is no test coverage here that isn't in the non-constexpr version. I suppose it wouldn't hurt to run these tests also outside of constexpr; I'll add that.

libcxx/test/std/utilities/function.objects/func.not_fn/not_fn.pass.cpp
548

Correct. The obj it's calling is actually callable with auto..., so it doesn't matter what types we pass to it. I don't know the original author's motivation for using std::string here; probably just it seemed convenient at the time.

I considered removing all this file's usage of <string>, but it seemed like an unnecessary rabbit hole at the moment.

Quuxplusone updated this revision to Diff 313771.Dec 26 2020, 8:41 PM
Quuxplusone marked an inline comment as done.

Run constexpr tests also in non-constexpr mode. Rebase on main.

Quuxplusone updated this revision to Diff 313787.Dec 27 2020, 7:35 AM

Poke the buildbot. (It's been stuck in "pre-merge checks" for 10 hours.)

Harbormaster completed remote builds in B83532: Diff 313771.Dec 27 2020, 5:11 PM
Harbormaster completed remote builds in B83540: Diff 313787.Dec 27 2020, 8:54 PM
Quuxplusone updated this revision to Diff 313860.Dec 28 2020, 8:46 AM

Fix a failing test (don't put constexpr on non-const methods in C++11).
If buildkite is green after this, imma push it.

Harbormaster completed remote builds in B83585: Diff 313860.Dec 28 2020, 9:34 AM
This revision was not accepted when it landed; it landed in state Needs Review.Dec 28 2020, 10:25 AM
Closed by commit rG7b00e9fae385: [libc++] [P1065] Constexpr invoke, reference_wrapper, mem_fn, not_fn… (authored by arthur.j.odwyer). · Explain Why
This revision was automatically updated to reflect the committed changes.
arthur.j.odwyer added a commit: rG7b00e9fae385: [libc++] [P1065] Constexpr invoke, reference_wrapper, mem_fn, not_fn….
arthur.j.odwyer mentioned this in rGdd756e3e8481: [libc++] Fix a test failure in 7b00e9fae3 (D93815)..Dec 28 2020, 10:57 AM
arthur.j.odwyer mentioned this in rGc0a2d3b90b3b: [libc++] Fix a test failure in 7b00e9fae3 (D93815)..Dec 28 2020, 3:47 PM
arthur.j.odwyer mentioned this in rGca1694b9d07c: Re-enable __cpp_lib_constexpr_functional..Jan 8 2021, 2:31 PM
zoecarver mentioned this in D92044: Implement P0655R1 visit<R>: Explicit Return Type for visit.Jan 11 2021, 4:55 PM

Revision Contents
Changeset List

PathSize
libcxx/
docs/
2 lines
2 lines
include/
19 lines
54 lines
14 lines
5 lines
test/
std/
language.support/
support.limits/
support.limits.general/
26 lines
30 lines
26 lines
32 lines
26 lines
26 lines
30 lines
utilities/
function.objects/
func.invoke/
279 lines
func.memfn/
9 lines
23 lines
23 lines
func.not_fn/
128 lines
func.search/
func.search.default/
21 lines
75 lines
utils/
7 lines

Diff 313871

View Options

libcxx/docs/Cxx2aStatusPaperStatus.csv

Loading...
View Options

libcxx/docs/FeatureTestMacroTable.rst

Loading...
View Options

libcxx/include/__functional_base

Loading...
View Options

libcxx/include/functional

Loading...
View Options

libcxx/include/type_traits

Loading...
View Options

libcxx/include/version

Loading...
View Options

libcxx/test/std/language.support/support.limits/support.limits.general/array.version.pass.cpp

Loading...
View Options

libcxx/test/std/language.support/support.limits/support.limits.general/functional.version.pass.cpp

Loading...
View Options

libcxx/test/std/language.support/support.limits/support.limits.general/iterator.version.pass.cpp

Loading...
View Options

libcxx/test/std/language.support/support.limits/support.limits.general/string_view.version.pass.cpp

Loading...
View Options

libcxx/test/std/language.support/support.limits/support.limits.general/tuple.version.pass.cpp

Loading...
View Options

libcxx/test/std/language.support/support.limits/support.limits.general/utility.version.pass.cpp

Loading...
View Options

libcxx/test/std/language.support/support.limits/support.limits.general/version.version.pass.cpp

Loading...
View Options

libcxx/test/std/utilities/function.objects/func.invoke/invoke_constexpr.pass.cpp

Loading...
View Options

libcxx/test/std/utilities/function.objects/func.memfn/member_data.pass.cpp

Loading...
View Options

libcxx/test/std/utilities/function.objects/func.memfn/member_function.pass.cpp

Loading...
View Options

libcxx/test/std/utilities/function.objects/func.memfn/member_function_const.pass.cpp

Loading...
View Options

libcxx/test/std/utilities/function.objects/func.not_fn/not_fn.pass.cpp

Loading...
View Options

libcxx/test/std/utilities/function.objects/func.search/func.search.default/default.pass.cpp

Loading...
View Options

libcxx/test/std/utilities/function.objects/func.search/func.search.default/default.pred.pass.cpp

Loading...
View Options

libcxx/utils/generate_feature_test_macro_components.py

Loading...