[libc++] Disentangle std::pointer_safety

[ldionne]

Summary

This patch gets rid of technical debt around std::pointer_safety which,
I claim, is entirely unnecessary. I don't think anybody has used
std::pointer_safety in actual code because we do not implement the
underlying garbage collection support. As such, I think it's entirely
fine to get rid of complex workarounds whose goals were to avoid breaking
the ABI back in 2017.

I'm putting this up both to get reviews and to discuss this proposal for
a breaking change. I think we should be comfortable with making these
tiny breaks if we are confident they won't hurt anyone, which I'm fairly
confident is the case here.

[• Quuxplusone]

libcxx/docs/ReleaseNotes.rst could use some line breaks, and say "scoped enum" rather than "enum class".
The comment // !C++03 should of course be worded more rigorously.
P2186 "Removing Garbage Collection Support" is relevant, and perhaps it would be appropriate to just wait until that paper's landed for C++2b and then rip out the whole thing? If anyone out there would object to wholesale-ripping-GC-out-of-all-modes, IMO it would be useful to bring that up now rather than later.

[ldionne]

Fix tests in c++03 mode, address Arthur's comments

[ldionne]

In D100410#2686931, @Quuxplusone wrote:

libcxx/docs/ReleaseNotes.rst could use some line breaks, and say "scoped enum" rather than "enum class".

Agreed and done.

The comment // !C++03 should of course be worded more rigorously.

I don't agree, what's unclear about it? You'd prefer !defined(_LIBCPP_CXX03_LANG)?

P2186 "Removing Garbage Collection Support" is relevant, and perhaps it would be appropriate to just wait until that paper's landed for C++2b and then rip out the whole thing? If anyone out there would object to wholesale-ripping-GC-out-of-all-modes, IMO it would be useful to bring that up now rather than later.

Hmm, thanks for bringing that paper up. The only question is whether we should rip it out in all standard modes or keep it in C++11/14/17/20 with an empty implementation. I'd be tempted to make the proposed (current) change now to remove tech debt (with no impact on anyone cause nobody cares), and then once P2186 lands, remove it from C++ > 20 only. That would maximize the conformance of our implementation.

[• Quuxplusone]

once P2186 lands, remove it from C++ > 20 only. That would maximize the conformance of our implementation

FWIW, I suspect that by the time p2186 lands, we'll be comfortable removing it in all modes; but I agree we don't need to solve that now.
LGTM, ship it!

libcxx/include/__memory/pointer_safety.h

50

You'd prefer !defined(_LIBCPP_CXX03_LANG)?

Yes, comparing the incidence of git grep 'endif.*C++03' versus git grep 'endif.*_LIBCPP_CXX03_LANG', I think the latter is better.

[cebowleratibm]

libcxx/include/__memory/pointer_safety.h

31

I suggest using attribute ((deprecated)) here.

[ldionne]

libcxx/include/__memory/pointer_safety.h

31

I was going to do it, but then I saw that the Standard didn't actually deprecate it. It seems that we haven't bothered. I think we should mark it as deprecated when (if) the Standard makes it so.

[mstorsjo]

@vvereschaka We're wondering if this commit is causing breakage in the llvm-clang-win-x-armv7l buildbot (e.g. https://lab.llvm.org/buildbot/#/builders/60/builds/3205), would you able to inspect and test in a similar setup/configuration? There are 13 tests failing; the substitutes-in-compile-flags.sh.cpp case should be handled by D102048, but we don't know exactly what caused the 12 other failing ones under std/atomics.

[mstorsjo]

In D100410#2750843, @mstorsjo wrote:

@vvereschaka We're wondering if this commit is causing breakage in the llvm-clang-win-x-armv7l buildbot (e.g. https://lab.llvm.org/buildbot/#/builders/60/builds/3205), would you able to inspect and test in a similar setup/configuration? There are 13 tests failing; the substitutes-in-compile-flags.sh.cpp case should be handled by D102048, but we don't know exactly what caused the 12 other failing ones under std/atomics.

Sorry for the noise, it seems like those atomics tests are working again in a newer test run. Nevermind...

libcxx/docs/ReleaseNotes.rst

- There has been several changes in the tuple constructors provided by libc++.
   properly initialize all the tuple elements - this is probably the culprit.
 
   In particular, the extension allowing tuples to be constructed from fewer
   elements than the number of elements in the tuple (in which case the remaining
   elements would be default-constructed) has been removed. See https://godbolt.org/z/sqozjd.
 
   Also, the extension allowing a tuple to be constructed from an array has been
   removed. See https://godbolt.org/z/5esqbW.
+
+- The ``std::pointer_safety`` utility and related functions are not available
+  in C++03 anymore. Furthermore, in other standard modes, it has changed from
+  a struct to a scoped enumeration, which is an ABI break. Finally, the
+  ``std::get_pointer_safety`` function was previously in the dylib, but it
+  is now defined as inline in the headers.
+
+  While this is technically both an API and an ABI break, we do not expect
+  ``std::pointer_safety`` to have been used at all in real code, since we
+  never implemented the underlying support for garbage collection.

libcxx/include/__config

 // Fix undefined behavior in how __hash_table stores its pointer types.
 #  define _LIBCPP_ABI_FIX_UNORDERED_NODE_POINTER_UB
 #  define _LIBCPP_ABI_FORWARD_LIST_REMOVE_NODE_POINTER_UB
 #  define _LIBCPP_ABI_FIX_UNORDERED_CONTAINER_SIZE_TYPE
 // Don't use a nullptr_t simulation type in C++03 instead using C++11 nullptr
 // provided under the alternate keyword __nullptr, which changes the mangling
 // of nullptr_t. This option is ABI incompatible with GCC in C++03 mode.
 #  define _LIBCPP_ABI_ALWAYS_USE_CXX11_NULLPTR
-// Define the `pointer_safety` enum as a C++11 strongly typed enumeration
-// instead of as a class simulating an enum. If this option is enabled
-// `pointer_safety` and `get_pointer_safety()` will no longer be available
-// in C++03.
-#  define _LIBCPP_ABI_POINTER_SAFETY_ENUM_TYPE
 // Define a key function for `bad_function_call` in the library, to centralize
 // its vtable and typeinfo to libc++ rather than having all other libraries
 // using that class define their own copies.
 #  define _LIBCPP_ABI_BAD_FUNCTION_CALL_KEY_FUNCTION
 // Enable optimized version of __do_get_(un)signed which avoids redundant copies.
 #  define _LIBCPP_ABI_OPTIMIZED_LOCALE_NUM_GET
 // Use the smallest possible integer type to represent the index of the variant.
 // Previously libc++ used "unsigned int" exclusively.

libcxx/include/__memory/pointer_safety.h

 #pragma GCC system_header
 #endif
 
 _LIBCPP_PUSH_MACROS
 #include <__undef_macros>
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-//enum class
-#if defined(_LIBCPP_ABI_POINTER_SAFETY_ENUM_TYPE)
-# ifndef _LIBCPP_CXX03_LANG
+#if !defined(_LIBCPP_CXX03_LANG)
+
 enum class pointer_safety : unsigned char {
   relaxed,
   preferred,
   strict
 };
-# endif
-#else
-struct _LIBCPP_TYPE_VIS pointer_safety
-{
-    enum __lx
-    {
-        relaxed,
-        preferred,
-        strict
-    };
-
-    __lx __v_;
 
-    _LIBCPP_INLINE_VISIBILITY
-    pointer_safety() : __v_() {}
-
-    _LIBCPP_INLINE_VISIBILITY
-    pointer_safety(__lx __v) : __v_(__v) {}
-    _LIBCPP_INLINE_VISIBILITY
-    operator int() const {return __v_;}
-};
-#endif
-
-#if !defined(_LIBCPP_ABI_POINTER_SAFETY_ENUM_TYPE) && \
-    defined(_LIBCPP_BUILDING_LIBRARY)
-_LIBCPP_FUNC_VIS pointer_safety get_pointer_safety() _NOEXCEPT;
-#else
-// This function is only offered in C++03 under ABI v1.
-# if !defined(_LIBCPP_ABI_POINTER_SAFETY_ENUM_TYPE) || !defined(_LIBCPP_CXX03_LANG)
 inline _LIBCPP_INLINE_VISIBILITY
 pointer_safety get_pointer_safety() _NOEXCEPT {
   return pointer_safety::relaxed;
 }
-# endif
-#endif
-
 
 _LIBCPP_FUNC_VIS void declare_reachable(void* __p);
 _LIBCPP_FUNC_VIS void declare_no_pointers(char* __p, size_t __n);
 _LIBCPP_FUNC_VIS void undeclare_no_pointers(char* __p, size_t __n);
 _LIBCPP_FUNC_VIS void* __undeclare_reachable(void* __p);
 
 template <class _Tp>
 inline _LIBCPP_INLINE_VISIBILITY
 _Tp*
 undeclare_reachable(_Tp* __p)
 {
     return static_cast<_Tp*>(__undeclare_reachable(__p));
 }
 
+#endif // !C++03
+
 _LIBCPP_END_NAMESPACE_STD
 
 _LIBCPP_POP_MACROS
 
 #endif // _LIBCPP___MEMORY_POINTER_SAFETY_H

libcxx/include/memory

 template <class T> struct hash;
 template <class T, class D> struct hash<unique_ptr<T, D> >;
 template <class T> struct hash<shared_ptr<T> >;
 
 template <class T, class Alloc>
   inline constexpr bool uses_allocator_v = uses_allocator<T, Alloc>::value;
 
 // Pointer safety
-enum class pointer_safety { relaxed, preferred, strict };
-void declare_reachable(void *p);
-template <class T> T *undeclare_reachable(T *p);
-void declare_no_pointers(char *p, size_t n);
-void undeclare_no_pointers(char *p, size_t n);
-pointer_safety get_pointer_safety() noexcept;
+enum class pointer_safety { relaxed, preferred, strict }; // since C++11
+void declare_reachable(void *p);                          // since C++11
+template <class T> T *undeclare_reachable(T *p);          // since C++11
+void declare_no_pointers(char *p, size_t n);              // since C++11
+void undeclare_no_pointers(char *p, size_t n);            // since C++11
+pointer_safety get_pointer_safety() noexcept;             // since C++11
 
 void* align(size_t alignment, size_t size, void*& ptr, size_t& space);
 
 }  // std
 
 */
 
 #include <__config>

libcxx/lib/abi/x86_64-apple-darwin.libcxxabi.v1.stable.exceptions.no_new_in_libcxx.abilist

 {'is_defined': True, 'name': '__ZNSt3__118__time_get_storageIwEC2ERKNS_12basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEE', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__118basic_stringstreamIcNS_11char_traitsIcEENS_9allocatorIcEEEaSEOS5_', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__118condition_variable10notify_allEv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__118condition_variable10notify_oneEv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__118condition_variable15__do_timed_waitERNS_11unique_lockINS_5mutexEEENS_6chrono10time_pointINS5_12system_clockENS5_8durationIxNS_5ratioILl1ELl1000000000EEEEEEE', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__118condition_variable4waitERNS_11unique_lockINS_5mutexEEE', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__118condition_variableD1Ev', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__118condition_variableD2Ev', 'type': 'FUNC'}
-{'is_defined': True, 'name': '__ZNSt3__118get_pointer_safetyEv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__118shared_timed_mutex11lock_sharedEv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__118shared_timed_mutex13unlock_sharedEv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__118shared_timed_mutex15try_lock_sharedEv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__118shared_timed_mutex4lockEv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__118shared_timed_mutex6unlockEv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__118shared_timed_mutex8try_lockEv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__118shared_timed_mutexC1Ev', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__118shared_timed_mutexC2Ev', 'type': 'FUNC'}

 {'is_defined': True, 'name': '___cxa_vec_delete2', 'type': 'I'}
 {'is_defined': True, 'name': '___cxa_vec_delete3', 'type': 'I'}
 {'is_defined': True, 'name': '___cxa_vec_dtor', 'type': 'I'}
 {'is_defined': True, 'name': '___cxa_vec_new', 'type': 'I'}
 {'is_defined': True, 'name': '___cxa_vec_new2', 'type': 'I'}
 {'is_defined': True, 'name': '___cxa_vec_new3', 'type': 'I'}
 {'is_defined': True, 'name': '___dynamic_cast', 'type': 'I'}
 {'is_defined': True, 'name': '___gxx_personality_v0', 'type': 'I'}
 No newline at end of file

libcxx/lib/abi/x86_64-unknown-linux-gnu.libcxxabi.v1.stable.exceptions.no_new_in_libcxx.abilist

 {'is_defined': True, 'name': '_ZNSt3__118__time_get_storageIwEC2ERKNS_12basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEE', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__118basic_stringstreamIcNS_11char_traitsIcEENS_9allocatorIcEEEaSEOS5_', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__118condition_variable10notify_allEv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__118condition_variable10notify_oneEv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__118condition_variable15__do_timed_waitERNS_11unique_lockINS_5mutexEEENS_6chrono10time_pointINS5_12system_clockENS5_8durationIxNS_5ratioILl1ELl1000000000EEEEEEE', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__118condition_variable4waitERNS_11unique_lockINS_5mutexEEE', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__118condition_variableD1Ev', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__118condition_variableD2Ev', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNSt3__118get_pointer_safetyEv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__118shared_timed_mutex11lock_sharedEv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__118shared_timed_mutex13unlock_sharedEv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__118shared_timed_mutex15try_lock_sharedEv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__118shared_timed_mutex4lockEv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__118shared_timed_mutex6unlockEv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__118shared_timed_mutex8try_lockEv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__118shared_timed_mutexC1Ev', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__118shared_timed_mutexC2Ev', 'type': 'FUNC'}

 {'is_defined': True, 'name': '_ZTv0_n24_NSt3__113basic_ostreamIcNS_11char_traitsIcEEED0Ev', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZTv0_n24_NSt3__113basic_ostreamIcNS_11char_traitsIcEEED1Ev', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZTv0_n24_NSt3__113basic_ostreamIwNS_11char_traitsIwEEED0Ev', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZTv0_n24_NSt3__113basic_ostreamIwNS_11char_traitsIwEEED1Ev', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZTv0_n24_NSt3__114basic_iostreamIcNS_11char_traitsIcEEED0Ev', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZTv0_n24_NSt3__114basic_iostreamIcNS_11char_traitsIcEEED1Ev', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZTv0_n24_NSt3__19strstreamD0Ev', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZTv0_n24_NSt3__19strstreamD1Ev', 'type': 'FUNC'}
 No newline at end of file

libcxx/src/memory.cpp

 {
 }
 
 void
 undeclare_no_pointers(char*, size_t)
 {
 }
 
-#if !defined(_LIBCPP_ABI_POINTER_SAFETY_ENUM_TYPE)
-pointer_safety get_pointer_safety() noexcept
-{
-    return pointer_safety::relaxed;
-}
-#endif
-
 void*
 __undeclare_reachable(void* p)
 {
     return p;
 }
 
 void*
 align(size_t alignment, size_t size, void*& ptr, size_t& space)

libcxx/test/libcxx/utilities/memory/util.dynamic.safety/get_pointer_safety_cxx03.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>
-
-// pointer_safety get_pointer_safety();
-
-#include <memory>
-#include <cassert>
-
-#include "test_macros.h"
-
-// libc++ doesn't offer std::pointer_safety in C++03 under the new ABI
-#if TEST_STD_VER < 11 && defined(_LIBCPP_ABI_POINTER_SAFETY_ENUM_TYPE)
-#define TEST_IS_UNSUPPORTED
-#endif
-
-#ifndef TEST_IS_UNSUPPORTED
-void test_pr26961() {
-  std::pointer_safety d;
-  d = std::get_pointer_safety();
-  assert(d == std::get_pointer_safety());
-}
-#endif
-
-int main(int, char**)
-{
-#ifndef TEST_IS_UNSUPPORTED
-  {
-    // Test that std::pointer_safety is still offered in C++03 under the old ABI.
-    std::pointer_safety r = std::get_pointer_safety();
-    assert(r == std::pointer_safety::relaxed ||
-           r == std::pointer_safety::preferred ||
-           r == std::pointer_safety::strict);
-  }
-  {
-    test_pr26961();
-  }
-#endif
-
-  return 0;
-}

libcxx/test/libcxx/utilities/memory/util.dynamic.safety/get_pointer_safety_new_abi.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>
-
-// pointer_safety get_pointer_safety();
-
-// The pointer_safety interface is no longer provided in C++03 in the new ABI.
-// XFAIL: c++03
-
-// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ABI_POINTER_SAFETY_ENUM_TYPE
-
-#include <memory>
-#include <cassert>
-
-#include "test_macros.h"
-
-int main(int, char**)
-{
-  {
-    static_assert(std::is_enum<std::pointer_safety>::value, "");
-    static_assert(!std::is_convertible<std::pointer_safety, int>::value, "");
-    static_assert(std::is_same<
-        std::underlying_type<std::pointer_safety>::type,
-        unsigned char
-    >::value, "");
-  }
-  {
-    std::pointer_safety r = std::get_pointer_safety();
-    assert(r == std::pointer_safety::relaxed ||
-           r == std::pointer_safety::preferred ||
-           r == std::pointer_safety::strict);
-  }
-
-  return 0;
-}

libcxx/test/std/utilities/memory/util.dynamic.safety/declare_no_pointers.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
 //
 //===----------------------------------------------------------------------===//
 
+// UNSUPPORTED: c++03
+
 // <memory>
 
 // void declare_no_pointers(char* p, size_t n);
 // void undeclare_no_pointers(char* p, size_t n);
 
 #include <memory>
 
 #include "test_macros.h"

libcxx/test/std/utilities/memory/util.dynamic.safety/declare_reachable.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
 //
 //===----------------------------------------------------------------------===//
 
+// UNSUPPORTED: c++03
+
 // <memory>
 
 // void declare_reachable(void* p);
 // template <class T> T* undeclare_reachable(T* p);
 
 #include <memory>
 #include <cassert>
 

libcxx/test/std/utilities/memory/util.dynamic.safety/get_pointer_safety.pass.cpp

 
 // UNSUPPORTED: c++03
 
 #include <memory>
 #include <cassert>
 
 #include "test_macros.h"
 
-
-void test_pr26961() {
-  std::pointer_safety d;
-  d = std::get_pointer_safety();
-  assert(d == std::get_pointer_safety());
-}
-
 int main(int, char**)
 {
   {
+    static_assert(std::is_enum<std::pointer_safety>::value, "");
+    static_assert(!std::is_convertible<std::pointer_safety, int>::value, "");
+    static_assert(std::is_same<
+        std::underlying_type<std::pointer_safety>::type,
+        unsigned char
+    >::value, "");
+  }
+  {
     std::pointer_safety r = std::get_pointer_safety();
     assert(r == std::pointer_safety::relaxed ||
            r == std::pointer_safety::preferred ||
            r == std::pointer_safety::strict);
   }
+  // Regression test for https://llvm.org/PR26961
   {
-    test_pr26961();
+    std::pointer_safety d;
+    d = std::get_pointer_safety();
+    assert(d == std::get_pointer_safety());
   }
 
   return 0;
 }