[libc++] std::to_address mustn't depend on P::element_type.

[ldionne]

Summary

This came out of @zoecarver's observation on D101404 that std::to_address wasn't working correctly with a type such as vector<int>::iterator whose element_type was being deduced wrongly. std::to_address actually shouldn't care about element_type at all.

[libc++] std::to_address mustn't depend on P::element_type.

Also, make sure it simulates an `auto` return type correctly.

[libc++] Prefer __to_address(it) over addressof(*it), for UBSAN's benefit.

UBSAN doesn't like `addressof(*it)` when `it` is positioned one-past-the-end
of an array, because that expression binds a reference to a non-object.
On the other hand, we have an overload of `__to_address` for raw pointer types
that Just Works; and we can expect that any user-provided fancy pointer type
will implement `operator->` so that `__to_address` will work just as well
(or better) than `operator*`.

This fixes a UBSAN failure in the previous commit's new test:

    iterator:1343:42: runtime error: reference binding to address 0x[...]
    with insufficient space for an object of type 'const char'
    [...]
      in std::__1::__wrap_iter<char const*>::operator->() const
      in std::__1::__to_address_helper<false>
      in std::__1::__to_address<std::__1::__wrap_iter<char const*> >
      in auto std::__1::to_address<std::__1::__wrap_iter<char const*> >
      in void test_container_iterators<std::__1::span<char const> >

The aforementioned failing test is here: https://buildkite.com/llvm-project/libcxx-ci/builds/2924

[• Quuxplusone]

libcxx/include/__memory/pointer_traits.h

184–192

I really want to pull this specialization up to right below the primary template, so that all the definitions of __to_address_helper are grouped together. However, that doesn't work, for some reason (probably somehow related to two-phase lookup).

[zoecarver]

This is great, thanks for working on this (and related)! I think this will fix some of the sfinae issues I was having with contiguous_iterator.

Do you know if __wap_iter plays nice with to_address after this patch? I suspect it will because it won't try to use element_type to determine the return type (it will just use operator->).

libcxx/include/__memory/pointer_traits.h

166–167

(Non-blocking) Could we get rid of __return_type and just make this auto everywhere? We'd have to change __choose_to_address to something like _IsValidExpansion<decltype(pointer_traits<_Pointer>::to_address(declval<const _Pointer&>()))>. I don't know if that would work but wdyt?

libcxx/test/std/utilities/memory/pointer.conversion/to_address.pass.cpp

65

Not sure if this is substantially different, what about a test where element_type = int*?

[• Quuxplusone]

Add libcxx-specific tests for _VSTD::__to_address.
Merge my new tests from D101404 — indeed I believe they pass now and D101404 might be obsolete (if still maybe containing some good ideas for tech debt reduction).

[• Quuxplusone]

libcxx/include/__memory/pointer_traits.h

166–167

I tried a bunch of simplifications locally and everything I tried eventually ran into some template-metaprogramming issue. Patches welcome (but test your idea locally because I bet it won't work for some obscure reason).

libcxx/test/std/utilities/memory/pointer.conversion/to_address.pass.cpp

65

The point of P5/P6 is just to test that auto to_address in fact returns a decayed type (distinguishing from decltype(auto) to_address which is effectively what we had before this patch). P5 tests the operator-> case; P6 tests the pointer_traits::to_address case.

[zoecarver]

Still LGTM. Thanks again for working on this!

Merge my new tests from D101404 — indeed I believe they pass now and D101404 might be obsolete (if still maybe containing some good ideas for tech debt reduction).

Great that these are passing now. I know it took a bit of iteration, but I think this is the "right" fix for the problem :) Always satisfying. Anyway, I agree it would probably be good to keep some of the cleanups from the other patch (but as a nfc commit or something).

libcxx/test/libcxx/utilities/memory/pointer.conversion/to_address.pass.cpp

2

Is this testing something substantially different than libcxx/test/std/utilities/memory/pointer.conversion/to_address.pass.cpp?

[• Quuxplusone]

libcxx/test/libcxx/utilities/memory/pointer.conversion/to_address.pass.cpp

2

This one tests _VSTD::__to_address in all modes; that one tests std::to_address in C++20 only.
I think it's important to have some relatively explicit coverage of __to_address pre-C++20 (now that its innermost guts are so significantly different in C++20-and-later), and I didn't see any existing coverage for __to_address anywhere.
This file is intentionally a cut-and-paste of the other one with only the underscores changed (and constexpr changed to TEST_CONSTEXPR and so on).

[• Quuxplusone]

[libc++] Prefer __to_address(it) over addressof(*it), for UBSAN's benefit.

UBSAN doesn't like `addressof(*it)` when `it` is positioned one-past-the-end
of an array, because that expression binds a reference to a non-object.
On the other hand, we have an overload of `__to_address` for raw pointer types
that Just Works; and we can expect that any user-provided fancy pointer type
will implement `operator->` so that `__to_address` will work just as well
(or better) than `operator*`.

This fixes a UBSAN failure in the previous commit's new test:

    iterator:1343:42: runtime error: reference binding to address 0x[...]
    with insufficient space for an object of type 'const char'
    [...]
      in std::__1::__wrap_iter<char const*>::operator->() const
      in std::__1::__to_address_helper<false>
      in std::__1::__to_address<std::__1::__wrap_iter<char const*> >
      in auto std::__1::to_address<std::__1::__wrap_iter<char const*> >
      in void test_container_iterators<std::__1::span<char const> >

[• Quuxplusone]

I'm stumped as to how to make my new to_address tests pass with both UBSan and _LIBCPP_DEBUG=1 (individually). I can get one or the other via different techniques, but haven't hit on the technique that will make them both (individually) work.
Actually I think I could do it with a partial specialization of pointer_traits<__wrap_iter<T>>; but I really resist that because that's just awful.

I would be willing to commit just the include/ diff, without the new tests, if that is ever needed in order to unblock @zoecarver or anyone. Just ping here, I guess.

[zoecarver]

libcxx/test/std/utilities/memory/pointer.conversion/to_address_std_iterators.pass.cpp

33

@Quuxplusone this is the reason the test is failing in debug mode, we can't dereference end. Do we ever really want to be dereferencing end anyway, isn't that UB? I don't think we need to test this, at least not in debug mode.

[zoecarver]

libcxx/test/std/utilities/memory/pointer.conversion/to_address_std_iterators.pass.cpp

33

Actually on second thought: we're not actually dereferencing anything here. I guess when the assertion was added to operator-> they didn't expect the caller of the operator to be using the pointer itself. So I feel like there's a good argument to remove the assertion all together. @EricWF, thoughts?

[• Quuxplusone]

libcxx/test/std/utilities/memory/pointer.conversion/to_address_std_iterators.pass.cpp

33

FYI @zoecarver, I believe you've got it now. But I don't think "remove the assertion" is correct. I think, (well, as you know I think) pointer_traits was just a dumb place to put this customization point (it should have been ADL like swap), but, I think the whole intention of letting the user customize to_address was precisely to give them a way to solve this exact problem. They have an operator-> that is unsafe to call on non-dereferenceable pointers, so they should customize to_address.

"They/the user" here means __wrap_iter, of course, which means us.

IOW, the answer is clearly to have libc++ provide a partial specialization of pointer_traits<__wrap_iter<U>> and maintain it forever. That's clearly the right thing to do. I resist it only because it's so, so dumb.

[zoecarver]

libcxx/test/std/utilities/memory/pointer.conversion/to_address_std_iterators.pass.cpp

33

FYI @zoecarver, I believe you've got it now. But I don't think "remove the assertion" is correct. I think, (well, as you know I think) pointer_traits was just a dumb place to put this customization point (it should have been ADL like swap), but, I think the whole intention of letting the user customize to_address was precisely to give them a way to solve this exact problem. They have an operator-> that is unsafe to call on non-dereferenceable pointers, so they should customize to_address.

"They/the user" here means __wrap_iter, of course, which means us.

IOW, the answer is clearly to have libc++ provide a partial specialization of pointer_traits<__wrap_iter<U>> and maintain it forever. That's clearly the right thing to do. I resist it only because it's so, so dumb.

Okay... I think you're right. Even though, as you said, this is super dumb.

However, we should still take this change, because it does fix something that was broken. We just also need to keep the specialization of to_address. I think we also need to add an element_type to the specialization of pointer_traits, otherwise it's ill formed.

[ldionne]

Thanks for fixing this. This LGTM, but we need to figure out how to write the tests correctly.

libcxx/test/std/utilities/memory/pointer.conversion/to_address_std_iterators.pass.cpp

33

It's actually not clear to me that this test is correct. I don't see why we're allowed to call to_address on a one-past-the-end iterator of a container. Imagine another implementation uses a different type of their own to implement e.g. std::vector<T>::iterator, then there would be no guarantee that we can use std::to_address on that at all, since vector<T>::iterator is only required to be a LegacyRandomAccessIterator (which doesn't even include to_address). Am I mistaken here?

[ldionne]

libcxx/test/std/utilities/memory/pointer.conversion/to_address_std_iterators.pass.cpp

33

Nevermind this, spoke with Zoe offline and we do indeed want to specialize pointer_traits for __wrap_iter.

[ldionne]

Commandeering since this has become pretty complicated. We have two issues:

1. std::to_address is broken and uses T::element_type when it shouldn't.
2. std::vector<T>::iterator is not a contiguous_iterator because it doesn't support std::to_address.

This patch fixes (1), and not (2).

[ldionne]

Mark failing test as UNSUPPORTED in debug mode, to be fixed once we fix std::vector::iterator.

[ldionne]

Poke BuildKite after trying to fix it

[zoecarver]

Ship it!

[ldionne]

The Debug Iterator job has passed, shipping. Thanks for your work on this Arthur (and others), let's fix __wrap_iter in D101404 after rebasing onto this patch.

[ldionne]

Fix Clang failure and add test case. Basically the issue was a subtle
case where we'd instantiate SFINAE checks too early even when the raw
pointer should have been selected right away.

That is not an issue when writing to_address to the spec, because it uses
auto-deduced return type instead, which doesn't require instantiating the
fancy pointer checking helper class during template argument deduction.

[ldionne]

libcxx/include/__memory/pointer_traits.h

207–208

Does anybody see an issue with doing this? I don't think this is detectable, so we should be conforming?

libcxx/test/std/utilities/memory/pointer.conversion/to_address.pass.cpp

105–115

This is the reduction of what triggered a build breakage in Clang yesterday.

[• Quuxplusone]

Discussed offline and gave Louis a couple more test cases to merge in here.

libcxx/include/__memory/pointer_traits.h

207–208

Is "this" the perfect-forwarding? I think it's reasonably undetectable, but also unneeded. I just changed this back to

auto
to_address(const _Pointer& __p) _NOEXCEPT
{
    return _VSTD::__to_address(__p);
}

locally, and it still passed all of our new tests with flying colors. So I strongly recommend either changing it back, or (if I'm missing something) adding a test case that "explains" the purpose of the forwarding reference.

[ldionne]

Apply Arthur's comments

libcxx/include/__memory/pointer_traits.h

 #ifndef _LIBCPP_CXX03_LANG
     typedef typename pointer_traits<_From>::template rebind<_To>        type;
 #else
     typedef typename pointer_traits<_From>::template rebind<_To>::other type;
 #endif
 };
 
 // to_address
 
-template <bool _UsePointerTraits> struct __to_address_helper;
-
-template <> struct __to_address_helper<true> {
-    template <class _Pointer>
-    using __return_type = decltype(pointer_traits<_Pointer>::to_address(_VSTD::declval<const _Pointer&>()));
-
-    template <class _Pointer>
-    _LIBCPP_CONSTEXPR
-    static __return_type<_Pointer>
-    __do_it(const _Pointer &__p) _NOEXCEPT { return pointer_traits<_Pointer>::to_address(__p); }
-};
-
-template <class _Pointer, bool _Dummy = true>
-using __choose_to_address = __to_address_helper<_IsValidExpansion<__to_address_helper<_Dummy>::template __return_type, _Pointer>::value>;
+template <class _Pointer, class = void>
+struct __to_address_helper;
 
 template <class _Tp>
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
-_Tp*
-__to_address(_Tp* __p) _NOEXCEPT
-{
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
+_Tp* __to_address(_Tp* __p) _NOEXCEPT {
     static_assert(!is_function<_Tp>::value, "_Tp is a function type");
     return __p;
 }
 
-template <class _Pointer>
-inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
-typename __choose_to_address<_Pointer>::template __return_type<_Pointer>
-__to_address(const _Pointer& __p) _NOEXCEPT
-{
-    return __choose_to_address<_Pointer>::__do_it(__p);
+// enable_if is needed here to avoid instantiating checks for fancy pointers on raw pointers
+template <class _Pointer, class = _EnableIf<!is_pointer<_Pointer>::value> >
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
+typename decay<decltype(__to_address_helper<_Pointer>::__call(declval<const _Pointer&>()))>::type
+__to_address(const _Pointer& __p) _NOEXCEPT {
+    return __to_address_helper<_Pointer>::__call(__p);
 }
 
-template <> struct __to_address_helper<false> {
-    template <class _Pointer>
-    using __return_type = typename pointer_traits<_Pointer>::element_type*;
+template <class _Pointer, class>
+struct __to_address_helper {
+    _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
+    static decltype(_VSTD::__to_address(declval<const _Pointer&>().operator->()))
+    __call(const _Pointer&__p) _NOEXCEPT {
+        return _VSTD::__to_address(__p.operator->());
+    }
+};
 
-    template <class _Pointer>
-    _LIBCPP_CONSTEXPR
-    static __return_type<_Pointer>
-    __do_it(const _Pointer &__p) _NOEXCEPT { return _VSTD::__to_address(__p.operator->()); }
+struct __to_address_helper<_Pointer, decltype((void)pointer_traits<_Pointer>::to_address(declval<const _Pointer&>()))> {
+    _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
+    static decltype(pointer_traits<_Pointer>::to_address(declval<const _Pointer&>()))
+    __call(const _Pointer&__p) _NOEXCEPT {
+        return pointer_traits<_Pointer>::to_address(__p);
+    }
 };
 
-
 #if _LIBCPP_STD_VER > 17
-template <class _Tp>
-inline _LIBCPP_INLINE_VISIBILITY constexpr
-_Tp*
-to_address(_Tp* __p) _NOEXCEPT
-{
-    static_assert(!is_function_v<_Tp>, "_Tp is a function type");
-    return __p;
-}
-
 template <class _Pointer>
 inline _LIBCPP_INLINE_VISIBILITY constexpr
-auto
-to_address(const _Pointer& __p) _NOEXCEPT
-{
+auto to_address(const _Pointer& __p) noexcept {
     return _VSTD::__to_address(__p);
 }
 #endif
 
 _LIBCPP_END_NAMESPACE_STD
 
 _LIBCPP_POP_MACROS
 
 #endif // _LIBCPP___MEMORY_POINTER_TRAITS_H

libcxx/include/iterator

#endif
         return *__i;
     }
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG pointer  operator->() const _NOEXCEPT
     {
 #if _LIBCPP_DEBUG_LEVEL == 2
         _LIBCPP_ASSERT(__get_const_db()->__dereferenceable(this),
                        "Attempted to dereference a non-dereferenceable iterator");
 #endif
-        return (pointer)_VSTD::addressof(*__i);
+        return _VSTD::__to_address(__i);
     }
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG __wrap_iter& operator++() _NOEXCEPT
     {
 #if _LIBCPP_DEBUG_LEVEL == 2
         _LIBCPP_ASSERT(__get_const_db()->__dereferenceable(this),
                        "Attempted to increment non-incrementable iterator");
 #endif
         ++__i;

libcxx/test/libcxx/utilities/memory/pointer.conversion/to_address.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <memory>
+
+// template <class T> constexpr T* __to_address(T* p) noexcept;
+// template <class Ptr> constexpr auto __to_address(const Ptr& p) noexcept;
+
+#include <memory>
+#include <cassert>
+#include "test_macros.h"
+
+struct Irrelevant;
+
+struct P1 {
+    using element_type = Irrelevant;
+    TEST_CONSTEXPR explicit P1(int *p) : p_(p) { }
+    TEST_CONSTEXPR int *operator->() const { return p_; }
+    int *p_;
+};
+
+struct P2 {
+    using element_type = Irrelevant;
+    TEST_CONSTEXPR explicit P2(int *p) : p_(p) { }
+    TEST_CONSTEXPR P1 operator->() const { return p_; }
+    P1 p_;
+};
+
+struct P3 {
+    TEST_CONSTEXPR explicit P3(int *p) : p_(p) { }
+    int *p_;
+};
+
+template<>
+struct std::pointer_traits<P3> {
+    static TEST_CONSTEXPR int *to_address(const P3& p) { return p.p_; }
+};
+
+struct P4 {
+    TEST_CONSTEXPR explicit P4(int *p) : p_(p) { }
+    int *operator->() const;  // should never be called
+    int *p_;
+};
+
+template<>
+struct std::pointer_traits<P4> {
+    static TEST_CONSTEXPR int *to_address(const P4& p) { return p.p_; }
+};
+
+struct P5 {
+    using element_type = Irrelevant;
+    int const* const& operator->() const;
+};
+
+struct P6 {};
+
+template<>
+struct std::pointer_traits<P6> {
+    static int const* const& to_address(const P6&);
+};
+
+// Taken from a build breakage caused in Clang
+namespace P7 {
+    template<typename T> struct CanProxy;
+    template<typename T>
+    struct CanQual {
+        CanProxy<T> operator->() const { return CanProxy<T>(); }
+    };
+    template<typename T>
+    struct CanProxy {
+        const CanProxy<T> *operator->() const { return nullptr; }
+    };
+} // namespace P7
+
+namespace P8 {
+    template<class T>
+    struct FancyPtrA {
+        using element_type = Irrelevant;
+        T *p_;
+        TEST_CONSTEXPR FancyPtrA(T *p) : p_(p) {}
+        T& operator*() const;
+        TEST_CONSTEXPR T *operator->() const { return p_; }
+    };
+    template<class T>
+    struct FancyPtrB {
+        T *p_;
+        TEST_CONSTEXPR FancyPtrB(T *p) : p_(p) {}
+        T& operator*() const;
+    };
+} // namespace P8
+
+template<class T>
+struct std::pointer_traits<P8::FancyPtrB<T> > {
+    static TEST_CONSTEXPR T *to_address(const P8::FancyPtrB<T>& p) { return p.p_; }
+};
+
+struct Incomplete;
+template<class T> struct Holder { T t; };
+
+
+TEST_CONSTEXPR_CXX14 bool test() {
+    int i = 0;
+    ASSERT_NOEXCEPT(std::__to_address(&i));
+    assert(std::__to_address(&i) == &i);
+    P1 p1(&i);
+    ASSERT_NOEXCEPT(std::__to_address(p1));
+    assert(std::__to_address(p1) == &i);
+    P2 p2(&i);
+    ASSERT_NOEXCEPT(std::__to_address(p2));
+    assert(std::__to_address(p2) == &i);
+    P3 p3(&i);
+    ASSERT_NOEXCEPT(std::__to_address(p3));
+    assert(std::__to_address(p3) == &i);
+    P4 p4(&i);
+    ASSERT_NOEXCEPT(std::__to_address(p4));
+    assert(std::__to_address(p4) == &i);
+
+    ASSERT_SAME_TYPE(decltype(std::__to_address(std::declval<int const*>())), int const*);
+    ASSERT_SAME_TYPE(decltype(std::__to_address(std::declval<P5>())), int const*);
+    ASSERT_SAME_TYPE(decltype(std::__to_address(std::declval<P6>())), int const*);
+
+    P7::CanQual<int>* p7 = nullptr;
+    assert(std::__to_address(p7) == nullptr);
+    ASSERT_SAME_TYPE(decltype(std::__to_address(p7)), P7::CanQual<int>*);
+
+    Holder<Incomplete> *p8_nil = nullptr;  // for C++03 compatibility
+    P8::FancyPtrA<Holder<Incomplete> > p8a = p8_nil;
+    assert(std::__to_address(p8a) == p8_nil);
+    ASSERT_SAME_TYPE(decltype(std::__to_address(p8a)), decltype(p8_nil));
+
+    P8::FancyPtrB<Holder<Incomplete> > p8b = p8_nil;
+    assert(std::__to_address(p8b) == p8_nil);
+    ASSERT_SAME_TYPE(decltype(std::__to_address(p8b)), decltype(p8_nil));
+
+    return true;
+}
+
+int main(int, char**) {
+    test();
+#if TEST_STD_VER >= 14
+    static_assert(test(), "");
+#endif
+    return 0;
+}

libcxx/test/libcxx/utilities/memory/pointer.conversion/to_address_std_iterators.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <memory>
+
+// template <class T> constexpr T* __to_address(T* p) noexcept;
+// template <class Ptr> constexpr auto __to_address(const Ptr& p) noexcept;
+
+#include <memory>
+
+#include <array>
+#include <cassert>
+#include <span>
+#include <string>
+#include <string_view>
+#include <valarray>
+#include <vector>
+#include "test_macros.h"
+
+template<class C>
+void test_container_iterators(C c)
+{
+    const C& cc = c;
+    assert(std::__to_address(c.begin()) == c.data());
+    assert(std::__to_address(c.end()) == c.data() + c.size());
+    assert(std::__to_address(cc.begin()) == cc.data());
+    assert(std::__to_address(cc.end()) == cc.data() + cc.size());
+}
+
+void test_valarray_iterators()
+{
+    std::valarray<int> v(100);
+    int *p = std::__to_address(std::begin(v));
+    int *q = std::__to_address(std::end(v));
+    assert(q - p == 100);
+}
+
+int main(int, char**) {
+    test_container_iterators(std::array<int, 3>());
+    test_container_iterators(std::vector<int>(3));
+    test_container_iterators(std::string("abc"));
+#if TEST_STD_VER >= 17
+    test_container_iterators(std::string_view("abc"));
+#endif
+#if TEST_STD_VER >= 20
+    test_container_iterators(std::span<const char>("abc"));
+#endif
+    test_valarray_iterators();
+
+    return 0;
+}

libcxx/test/std/utilities/memory/pointer.conversion/to_address.pass.cpp

 
 // template <class T> constexpr T* to_address(T* p) noexcept;
 // template <class Ptr> constexpr auto to_address(const Ptr& p) noexcept;
 
 #include <memory>
 #include <cassert>
 #include "test_macros.h"
 
-class P1
-{
-public:
-    using element_type = int;
+struct Irrelevant;
 
-    constexpr explicit P1(int* p)
-    : p_(p) { }
-
-    constexpr int* operator->() const noexcept
-    { return p_; }
-
-private:
-    int* p_;
+struct P1 {
+    using element_type = Irrelevant;
+    constexpr explicit P1(int *p) : p_(p) { }
+    constexpr int *operator->() const { return p_; }
+    int *p_;
 };
 
-class P2
-{
-public:
-    using element_type = int;
-
-    constexpr explicit P2(int* p)
-    : p_(p) { }
-
-    constexpr P1 operator->() const noexcept
-    { return p_; }
-
-private:
+struct P2 {
+    using element_type = Irrelevant;
+    constexpr explicit P2(int *p) : p_(p) { }
+    constexpr P1 operator->() const { return p_; }
     P1 p_;
 };
 
-class P3
-{
-public:
-    constexpr explicit P3(int* p)
-    : p_(p) { }
+struct P3 {
+    constexpr explicit P3(int *p) : p_(p) { }
+    int *p_;
+};
 
-    constexpr int* get() const noexcept
-    { return p_; }
+template<>
+struct std::pointer_traits<P3> {
+    static constexpr int *to_address(const P3& p) { return p.p_; }
+};
 
-private:
-    int* p_;
+struct P4 {
+    constexpr explicit P4(int *p) : p_(p) { }
+    int *operator->() const;  // should never be called
+    int *p_;
 };
 
-namespace std
-{
 template<>
-struct pointer_traits<::P3>
-{
-    static constexpr int* to_address(const ::P3& p) noexcept
-    { return p.get(); }
+struct std::pointer_traits<P4> {
+    static constexpr int *to_address(const P4& p) { return p.p_; }
 };
-}
-
-class P4
-{
-public:
-    constexpr explicit P4(int* p)
-    : p_(p) { }
 
-    constexpr int* operator->() const noexcept
-    { return nullptr; }
+struct P5 {
+    using element_type = Irrelevant;
+    int const* const& operator->() const;
+};
 
-    constexpr int* get() const noexcept
-    { return p_; }
+struct P6 {};
 
-private:
-    int* p_;
+template<>
+struct std::pointer_traits<P6> {
+    static int const* const& to_address(const P6&);
 };
 
-namespace std
-{
-template<>
-struct pointer_traits<::P4>
-{
-    constexpr static int* to_address(const ::P4& p) noexcept
-    { return p.get(); }
+// Taken from a build breakage caused in Clang
+namespace P7 {
+    template<typename T> struct CanProxy;
+    template<typename T>
+    struct CanQual {
+        CanProxy<T> operator->() const { return CanProxy<T>(); }
+    };
+    template<typename T>
+    struct CanProxy {
+        const CanProxy<T> *operator->() const { return nullptr; }
+    };
+} // namespace P7
+
+namespace P8 {
+    template<class T>
+    struct FancyPtrA {
+        using element_type = Irrelevant;
+        T *p_;
+        TEST_CONSTEXPR FancyPtrA(T *p) : p_(p) {}
+        T& operator*() const;
+        TEST_CONSTEXPR T *operator->() const { return p_; }
+    };
+    template<class T>
+    struct FancyPtrB {
+        T *p_;
+        TEST_CONSTEXPR FancyPtrB(T *p) : p_(p) {}
+        T& operator*() const;
+    };
+} // namespace P8
+
+template<class T>
+struct std::pointer_traits<P8::FancyPtrB<T> > {
+    static TEST_CONSTEXPR T *to_address(const P8::FancyPtrB<T>& p) { return p.p_; }
 };
-}
 
-int n = 0;
-static_assert(std::to_address(&n) == &n);
+struct Incomplete;
+template<class T> struct Holder { T t; };
+
 
 constexpr bool test() {
     int i = 0;
     ASSERT_NOEXCEPT(std::to_address(&i));
     assert(std::to_address(&i) == &i);
     P1 p1(&i);
     ASSERT_NOEXCEPT(std::to_address(p1));
     assert(std::to_address(p1) == &i);
     P2 p2(&i);
     ASSERT_NOEXCEPT(std::to_address(p2));
     assert(std::to_address(p2) == &i);
     P3 p3(&i);
     ASSERT_NOEXCEPT(std::to_address(p3));
     assert(std::to_address(p3) == &i);
     P4 p4(&i);
     ASSERT_NOEXCEPT(std::to_address(p4));
     assert(std::to_address(p4) == &i);
 
+    ASSERT_SAME_TYPE(decltype(std::to_address(std::declval<int const*>())), int const*);
+    ASSERT_SAME_TYPE(decltype(std::to_address(std::declval<P5>())), int const*);
+    ASSERT_SAME_TYPE(decltype(std::to_address(std::declval<P6>())), int const*);
+
+    P7::CanQual<int>* p7 = nullptr;
+    assert(std::to_address(p7) == nullptr);
+    ASSERT_SAME_TYPE(decltype(std::to_address(p7)), P7::CanQual<int>*);
+
+    Holder<Incomplete> *p8_nil = nullptr;  // for C++03 compatibility
+    P8::FancyPtrA<Holder<Incomplete> > p8a = p8_nil;
+    assert(std::to_address(p8a) == p8_nil);
+    ASSERT_SAME_TYPE(decltype(std::to_address(p8a)), decltype(p8_nil));
+
+    P8::FancyPtrB<Holder<Incomplete> > p8b = p8_nil;
+    assert(std::to_address(p8b) == p8_nil);
+    ASSERT_SAME_TYPE(decltype(std::to_address(p8b)), decltype(p8_nil));
+
     return true;
 }
 
 int main(int, char**) {
     test();
     static_assert(test());
     return 0;
 }

libcxx/test/std/utilities/memory/pointer.conversion/to_address_std_iterators.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// <memory>
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
+// TODO: We should enable this test in Debug mode once we fix __wrap_iter
+//       to be a proper contiguous_iterator.
+// UNSUPPORTED: LIBCXX-DEBUG-FIXME
+
+// template <class T> constexpr T* to_address(T* p) noexcept;
+// template <class Ptr> constexpr auto to_address(const Ptr& p) noexcept;
+
+#include <memory>
+
+#include <array>
+#include <cassert>
+#include <span>
+#include <string>
+#include <string_view>
+#include <valarray>
+#include <vector>
+#include "test_macros.h"
+
+template<class C>
+void test_container_iterators(C c)
+{
+    const C& cc = c;
+    assert(std::to_address(c.begin()) == c.data());
+    assert(std::to_address(c.end()) == c.data() + c.size());
+    assert(std::to_address(cc.begin()) == cc.data());
+    assert(std::to_address(cc.end()) == cc.data() + cc.size());
+}
+
+void test_valarray_iterators()
+{
+    std::valarray<int> v(100);
+    int *p = std::to_address(std::begin(v));
+    int *q = std::to_address(std::end(v));
+    assert(q - p == 100);
+}
+
+int main(int, char**) {
+    test_container_iterators(std::array<int, 3>());
+    test_container_iterators(std::vector<int>(3));
+    test_container_iterators(std::string("abc"));
+    test_container_iterators(std::string_view("abc"));
+    test_container_iterators(std::span<const char>("abc"));
+    test_valarray_iterators();
+
+    return 0;
+}