This is an archive of the discontinued LLVM Phabricator instance.

Add constexpr to pair
ClosedPublic

Authored by miscco on May 26 2020, 8:11 AM.

Details

Reviewers
ldionne
Group Reviewers
Restricted Project
Commits
rG737a4501e815: Add constexpr to pair

Diff Detail

Event Timeline

miscco created this revision.May 26 2020, 8:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 26 2020, 8:11 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

I somehow missed to add the description to the PR so here it is:

This implements the changes to utility as described by the misc_constexpr paper.

While there I tried to overhaul the tests to get rid of the local classes that should be able to be replaced by the archetypes

Ping, a second *pair* of eyes wouldn't hurt.

I am terrible at jokes ...

miscco marked 3 inline comments as done.Jun 10 2020, 11:59 PM

Took the opportunity to look through this again. Mostly nits about the tests

libcxx/test/std/utilities/utility/pairs/pairs.pair/assign_rv_pair.pass.cpp
39

I believe I have to check this one here, It seems moving to ConstexprTestTypes::MoveOnly should suffice

libcxx/test/std/utilities/utility/pairs/pairs.pair/assign_rv_pair_U_V.pass.cpp
43

I am really not too happy with this dance. I guess one should add one additional section for C++20 only

libcxx/test/std/utilities/utility/pairs/pairs.pair/swap.pass.cpp
24 ↗(On Diff #266217)

Could we remove those by the way? I do not see any use for them

Could someone have a look at this?

miscco updated this revision to Diff 272376.Jun 22 2020, 3:52 AM

Updated tests

miscco updated this revision to Diff 272377.Jun 22 2020, 3:52 AM

Missed some changes

Mostly LGTM with nits.

libcxx/test/std/utilities/utility/pairs/pairs.pair/assign_rv_pair.pass.cpp
27

This isn't used.

libcxx/test/std/utilities/utility/pairs/pairs.pair/assign_rv_pair_U_V.pass.cpp
43

I think this is acceptable, we do it in a bunch of other places for testing constexpr. I do agree that something in the test runner would be better, but eh.

libcxx/test/std/utilities/utility/pairs/pairs.pair/swap.pass.cpp
24 ↗(On Diff #266217)

Do you mean remove S altogether? It's used below.

miscco added inline comments.Sep 1 2020, 12:13 PM
libcxx/test/std/utilities/utility/pairs/pairs.pair/assign_rv_pair.pass.cpp
27

True, will remove

libcxx/test/std/utilities/utility/pairs/pairs.pair/swap.pass.cpp
24 ↗(On Diff #266217)

I think that was from a previous revision where I did not yet removed those two lines

S * operator& () { assert(false); return this; }
S const * operator& () const { assert(false); return this; }
miscco updated this revision to Diff 289378.Sep 2 2020, 2:37 AM

Remove the unused member function from CountAssign

miscco updated this revision to Diff 289379.Sep 2 2020, 2:41 AM

Please use all the changes

miscco updated this revision to Diff 289380.Sep 2 2020, 2:42 AM

Once again into the breach

miscco updated this revision to Diff 289391.Sep 2 2020, 4:11 AM
  • Fix formatting of meow.version.pass.cpp
ldionne accepted this revision.Sep 2 2020, 7:07 AM

Thanks!

This revision is now accepted and ready to land.Sep 2 2020, 7:07 AM
This revision was landed with ongoing or failed builds.Sep 2 2020, 12:22 PM
Closed by commit rG737a4501e815: Add constexpr to pair (authored by miscco). · Explain Why
This revision was automatically updated to reflect the committed changes.