Page MenuHomePhabricator

Add partial implementation of std::to_address() as llvm::to_address()
ClosedPublic

Authored by dsanders on Feb 19 2019, 5:54 PM.

Details

Summary

Following on from the review for D58088, this patch provides the
prerequisite to_address() implementation that's needed to have
pointer_iterator support unique_ptr.

'typename Ptr::element_type *' should be replaced with 'auto' once we move
to C++14 to better align with the C++20 declaration. Until then, this
implementation is limited to fancy pointers with an element_type typedef.
Also, this implementation can be removed once we move to C++20 where it's
defined as std::to_addres()

The std::pointer_traits<>::to_address(p) variations of these overloads has
not been implemented.

Diff Detail

Repository
rL LLVM

Event Timeline

dsanders created this revision.Feb 19 2019, 5:54 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 19 2019, 5:54 PM
dexonsmith added inline comments.Feb 19 2019, 6:05 PM
include/llvm/ADT/STLExtras.h
1589–1592 ↗(On Diff #187480)

Can you use decltype(std::declval<Ptr>().operator->()) instead of typename Ptr::element_type * to avoid requiring an element_type typedef?

dblaikie added inline comments.Feb 19 2019, 9:03 PM
include/llvm/ADT/STLExtras.h
1589–1592 ↗(On Diff #187480)

Yep, or late bound return:

auto to_address(const Ptr &P) -> decltype(P.operator->()) { return P.operator->(); }

Also probably leave off the noexcept - LLVM doesn't build with exceptions anyway. So there's not a lot/much point in littering noexcept everywhere.

dsanders updated this revision to Diff 187605.Feb 20 2019, 9:58 AM
dsanders marked an inline comment as done.

Late bound return, remove noexcept, and update comments to match

include/llvm/ADT/STLExtras.h
1589–1592 ↗(On Diff #187480)

I've gone with the late bound return, partly because it's a little shorter and partly because it means we just drop the -> onwards for C++14.

Also probably leave off the noexcept - LLVM doesn't build with exceptions anyway. So there's not a lot/much point in littering noexcept everywhere.

Sure. I only included it to match the std::to_address() declaration we're implementing. As you say, it's not doing anything for LLVM

dblaikie accepted this revision.Feb 20 2019, 10:04 AM

Looks good - thanks!

This revision is now accepted and ready to land.Feb 20 2019, 10:04 AM
This revision was automatically updated to reflect the committed changes.
glenjofe added inline comments.
llvm/trunk/include/llvm/ADT/STLExtras.h
1588

The fancy pointer overload should return to_address(p.operator->()) but this returns p.operator->().

glenjofe added inline comments.Feb 20 2019, 5:02 PM
llvm/trunk/include/llvm/ADT/STLExtras.h
1582–1583

Is this necessary because you do not have access to __to_raw_pointer() in libc++? That is available in pre-C++20. The actual implementation of std::to_address() in libc++ is what I implemented in __to_raw_pointer() in D35470.

dblaikie added inline comments.Feb 21 2019, 9:23 AM
llvm/trunk/include/llvm/ADT/STLExtras.h
1582–1583

Yeah, this code needs to be portable to other compilers and standard libraries (LLVM can be built with a bunch of different compilers).

1588

What sort of cases would this support (trying to think of what test coverage to add)?

I guess a multiply-indirect smart pointer? Pretty rare beast, not sure we'd really care/need to support that for LLVM's uses?