[libcxx-commits] [libcxx] fe0e86e - [libc++] Rewrite std::to_address to avoid relying on element_type

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Thu May 6 07:15:13 PDT 2021


Author: Louis Dionne
Date: 2021-05-06T10:14:11-04:00
New Revision: fe0e86e6026f79e0b18f877196fbddd1d9e140d8

URL: https://github.com/llvm/llvm-project/commit/fe0e86e6026f79e0b18f877196fbddd1d9e140d8
DIFF: https://github.com/llvm/llvm-project/commit/fe0e86e6026f79e0b18f877196fbddd1d9e140d8.diff

LOG: [libc++] Rewrite std::to_address to avoid relying on element_type

This is a rough reapplication of the change that fixed std::to_address
to avoid relying on element_type (da456167). It is somewhat different
because the fix to avoid breaking Clang (which caused it to be reverted
in 347f69c55) was a bit more involved.

Differential Revision: https://reviews.llvm.org/D101638

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

Modified: 
    libcxx/include/__memory/pointer_traits.h
    libcxx/include/iterator
    libcxx/test/std/utilities/memory/pointer.conversion/to_address.pass.cpp

Removed: 
    


################################################################################
diff  --git a/libcxx/include/__memory/pointer_traits.h b/libcxx/include/__memory/pointer_traits.h
index 439c658b50766..6730066edc31c 100644
--- a/libcxx/include/__memory/pointer_traits.h
+++ b/libcxx/include/__memory/pointer_traits.h
@@ -164,64 +164,46 @@ struct __rebind_pointer {
 
 // 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>
-    _LIBCPP_CONSTEXPR
-    static __return_type<_Pointer>
-    __do_it(const _Pointer &__p) _NOEXCEPT { return _VSTD::__to_address(__p.operator->()); }
+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>
+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

diff  --git a/libcxx/include/iterator b/libcxx/include/iterator
index 1d308a2fa7bd7..687c641417c6a 100644
--- a/libcxx/include/iterator
+++ b/libcxx/include/iterator
@@ -1356,7 +1356,7 @@ public:
         _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
     {

diff  --git a/libcxx/test/libcxx/utilities/memory/pointer.conversion/to_address.pass.cpp b/libcxx/test/libcxx/utilities/memory/pointer.conversion/to_address.pass.cpp
new file mode 100644
index 0000000000000..edc51c3d7a5dc
--- /dev/null
+++ b/libcxx/test/libcxx/utilities/memory/pointer.conversion/to_address.pass.cpp
@@ -0,0 +1,149 @@
+//===----------------------------------------------------------------------===//
+//
+// 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;
+}

diff  --git a/libcxx/test/libcxx/utilities/memory/pointer.conversion/to_address_std_iterators.pass.cpp b/libcxx/test/libcxx/utilities/memory/pointer.conversion/to_address_std_iterators.pass.cpp
new file mode 100644
index 0000000000000..5eed12d19c072
--- /dev/null
+++ b/libcxx/test/libcxx/utilities/memory/pointer.conversion/to_address_std_iterators.pass.cpp
@@ -0,0 +1,56 @@
+//===----------------------------------------------------------------------===//
+//
+// 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;
+}

diff  --git a/libcxx/test/std/utilities/memory/pointer.conversion/to_address.pass.cpp b/libcxx/test/std/utilities/memory/pointer.conversion/to_address.pass.cpp
index 26eba0e06cffe..0a2bdf11559ee 100644
--- a/libcxx/test/std/utilities/memory/pointer.conversion/to_address.pass.cpp
+++ b/libcxx/test/std/utilities/memory/pointer.conversion/to_address.pass.cpp
@@ -17,110 +17,133 @@
 #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);
-
-  return true;
+    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;
+    test();
+    static_assert(test());
+    return 0;
 }

diff  --git a/libcxx/test/std/utilities/memory/pointer.conversion/to_address_std_iterators.pass.cpp b/libcxx/test/std/utilities/memory/pointer.conversion/to_address_std_iterators.pass.cpp
new file mode 100644
index 0000000000000..45d034c5c232d
--- /dev/null
+++ b/libcxx/test/std/utilities/memory/pointer.conversion/to_address_std_iterators.pass.cpp
@@ -0,0 +1,58 @@
+//===----------------------------------------------------------------------===//
+//
+// 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;
+}


        


More information about the libcxx-commits mailing list