This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] Implement P0318R1: unwrap_ref_decay and unwrap_reference
ClosedPublic

Authored by ldionne on Nov 13 2018, 11:49 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

ldionne created this revision.Nov 13 2018, 11:49 AM
ldionne updated this revision to Diff 173915.Nov 13 2018, 12:37 PM

Include unwrap_reference_t

EricWF accepted this revision.Nov 13 2018, 1:45 PM

LGTM.

This revision is now accepted and ready to land.Nov 13 2018, 1:45 PM
ldionne added inline comments.Nov 13 2018, 1:48 PM
libcxx/include/functional
2536 ↗(On Diff #173915)

Is this a viable implementation strategy? The nice part is that this allows us to reuse these metafunctions to implement make_tuple and make_pair before C++20, but IDK whether the standard allows us to bring these names into namespace std with a using declaration?

Is that observable (e.g. through ADL associated namespaces)?

libcxx/include/utility
619 ↗(On Diff #173915)

This is hardly the "right" place to implement this, however this is the only way I could find not to create another header.

EricWF requested changes to this revision.Nov 13 2018, 9:20 PM
EricWF added inline comments.
libcxx/include/functional
2536 ↗(On Diff #173915)

After more thought: No, this is not viable.

I believe these are technically customization points for users, so it needs to have the same declaration as in the standard, not just the same spelling.

Bummer.

I guess the best route is to have __foo for use pre-C++20, and the we can implement foo in terms of __foo?

This revision now requires changes to proceed.Nov 13 2018, 9:20 PM
EricWF added inline comments.Nov 13 2018, 9:21 PM
libcxx/include/utility
619 ↗(On Diff #173915)

Meh. Just put it where it fits best.

ldionne added inline comments.Nov 19 2018, 11:02 AM
libcxx/include/functional
2536 ↗(On Diff #173915)

I believe these are technically customization points for users, so it needs to have the same declaration as in the standard, not just the same spelling.

We need to fix that.. Only things that are specifically designed to be customization points should be customizable by users. We need a way to tag things in the standard as being a customization point.

ldionne updated this revision to Diff 174665.Nov 19 2018, 12:08 PM

Implement as written in the spec

I agree with your concerns about this living in <utility>, but as long as people can get it by including <type_traits>, then we're good.

libcxx/include/functional
2536 ↗(On Diff #173915)

I guess the best route is to have __foo for use pre-C++20, and the we can implement foo in terms of __foo?

We use this approach in lots of other places.

libcxx/include/utility
631 ↗(On Diff #174665)

You've still got a C++20 here :-)

ldionne updated this revision to Diff 175579.Nov 27 2018, 2:17 PM

Address Marshall's comment.

EricWF accepted this revision.Nov 30 2018, 3:44 PM

LGTM.

This revision is now accepted and ready to land.Nov 30 2018, 3:44 PM
This revision was automatically updated to reflect the committed changes.