[libc++] Implement P2186R2 (Remove Garbage Collection)

[• Quuxplusone]

This approach seems acceptable to me because I don't imagine that anyone really cares about the pointer safety facilities. However, it seems to deliberately take the bad middle path between two better extremes:

• If we believe the rationale in the paper (and my own prejudices), I suggest that we should simplify libc++ by aggressively removing the facilities from all C++ versions, removing the ifdefs, removing the tests, removing the detail header, and release-noting it. (And removing the abilist entries and memory.cpp stuff? Or is that a bridge too far?)
• If we truly believe that there exists C++20 code in the wild that is using these facilities, then unconditionally dropping them from C++2b seems user-hostile. We should use the usual libc++ approach of allowing users to opt back in to removed features via _LIBCPP_ENABLE_CXX23_REMOVED_FEATURES/_LIBCPP_ENABLE_CXX23_REMOVED_POINTER_SAFETY.

libcxx/include/__memory/pointer_safety.h

0

Pre-existing: This comment should just be

#endif // !defined(_LIBCPP_CXX03_LANG) && _LIBCPP_STD_VER <= 20

0

These four functions are defined in src/memory.cpp, and thus in the abilists. Should we do anything special with them in either of those places? (I suspect "do nothing special" is correct, but I'm not sure.)

0

Please update the synopsis in <memory> with // removed in C++23 comments, too (or delete those lines entirely, in the event that we decide on the aggressive-removal option).

[philnik]

I'm going with option 1 right now. If anybody thinks there are users I can revert the changes later.

• Removed pointer_safety

[ldionne]

At first sight, I'm OK with this removal. I, too, can't imagine anyone relying on that code. However, I think we should still keep those symbols around because we have a hard policy of never removing symbols exported from the dylib (except in special circumstances where the symbol is also defined in any TU that uses the said function, like a function template that would end up being exported from the dylib by mistake).

Long story short, I would create a legacy_pointer_safety.cpp file and shove declare_reachable & friends in it. I would add a comment explaining that we don't ship that feature anymore, but we're still providing the symbols for strict backwards ABI compatibility.

This way, if someone has compiled code that (for some probably bad) reason relied on those symbols, their program will keep working when libc++ is updated from under their feet (e.g. when you update your OS on any platform that uses libc++). If someone used to rely on those and tries to compile their code against a new libc++, they will have to get rid of those references because the library won't provide the declarations anymore - but I think it's reasonable to request some minimal action if folks are recompiling their code. Finally, this solution is very low maintenance for us.

[• Quuxplusone]

LGTM mod Louis's comments! To fix the CI redness, you'll need to git rm the autogenerated header test file that is no longer being generated.
I think the new release note should go like this:

API Changes
-----------

The declarations of functions `declare_reachable`, `undeclare_reachable`, `declare_no_pointers`,
`undeclare_no_pointers`, and `get_pointer_safety` have been removed not only from C++2b but
from all modes. Their symbols are still provided by the dynamic library for the benefit of
existing compiled code. All of these functions have always behaved as no-ops.

(Technically get_pointer_safety is not a "no-op"; and undeclare_reachable is a template, not a function; but I don't think those technicalities are worth complicating the message.)

[philnik]

• Added legacy_pointer_safety.cpp for ABI compatiblilty

[philnik]

How do I add the symbols back into the library?

[ldionne]

libcxx/src/legacy_pointer_safety.cpp

1

Please don't add the name of the file. We're trying to move away from that.

12–13

17–20

I believe that should fix your ABI list issues.

[philnik]

Applied comments

[• Quuxplusone]

libcxx/src/legacy_pointer_safety.cpp

13

Remove bogus (if harmless) trailing dot in wg21.link./

[philnik]

• Removed .

[ldionne]

Thanks.

Just for posterity, this doesn't create a precedent for backporting removals to all standard modes when a paper lands. This case is special -- our support for garbage collection has always been a joke and I think the library is better for not presenting this basically unimplemented API to users. At least we don't pretend that we implement something reasonable.

[philnik]

I don't have commit permissions, so someone else has to commit it for me. "Nikolas Klauser" <[e-mail]>

[Mordante]

@philnik FYI There was a trivial merge conflict in the release notes.

[Pawday]

Why it doesn't rely on _LIBCPP_STD_VER?

[philnik]

In D112869#4152335, @Pawday wrote:

Why it doesn't rely on _LIBCPP_STD_VER?

As discussed in the paper, there is no known implementation of the strict pointer safety facilities, which means that every use of this is a no-op, and therefore probably wrong. Pretty much the only code you can find is inside tests or a standard library: https://sourcegraph.com/search?q=context:global+declare_reachable+file:%5C.cpp+-file:en%5C.cppreference%5C.com+-file:declare_reachable%5C.pass%5C.cpp+-file:memory%5C.cpp+-file:legacy_pointer_safety%5C.cpp&patternType=standard&sm=1&groupBy=repo

[Pawday]

In D112869#4152384, @philnik wrote:

In D112869#4152335, @Pawday wrote:

Why it doesn't rely on _LIBCPP_STD_VER?

As discussed in the paper, there is no known implementation of the strict pointer safety facilities, which means that every use of this is a no-op, and therefore probably wrong. Pretty much the only code you can find is inside tests or a standard library: https://sourcegraph.com/search?q=context:global+declare_reachable+file:%5C.cpp+-file:en%5C.cppreference%5C.com+-file:declare_reachable%5C.pass%5C.cpp+-file:memory%5C.cpp+-file:legacy_pointer_safety%5C.cpp&patternType=standard&sm=1&groupBy=repo

Does this mean if i set my compiler to -std=c++14 for example, and will include "memory" i would not get the full c++ 14 support (with std::declare_reachable for example)?

[philnik]

In D112869#4152409, @Pawday wrote:

In D112869#4152384, @philnik wrote:

In D112869#4152335, @Pawday wrote:

Why it doesn't rely on _LIBCPP_STD_VER?

As discussed in the paper, there is no known implementation of the strict pointer safety facilities, which means that every use of this is a no-op, and therefore probably wrong. Pretty much the only code you can find is inside tests or a standard library: https://sourcegraph.com/search?q=context:global+declare_reachable+file:%5C.cpp+-file:en%5C.cppreference%5C.com+-file:declare_reachable%5C.pass%5C.cpp+-file:memory%5C.cpp+-file:legacy_pointer_safety%5C.cpp&patternType=standard&sm=1&groupBy=repo

Does this mean if i set my compiler to -std=c++14 for example, and will include "memory" i would not get the full c++ 14 support (with std::declare_reachable for example)?

Technically yes, but I don't think anybody cares.

[ldionne]

In D112869#4152409, @Pawday wrote:

In D112869#4152384, @philnik wrote:

In D112869#4152335, @Pawday wrote:

Why it doesn't rely on _LIBCPP_STD_VER?

As discussed in the paper, there is no known implementation of the strict pointer safety facilities, which means that every use of this is a no-op, and therefore probably wrong. Pretty much the only code you can find is inside tests or a standard library: https://sourcegraph.com/search?q=context:global+declare_reachable+file:%5C.cpp+-file:en%5C.cppreference%5C.com+-file:declare_reachable%5C.pass%5C.cpp+-file:memory%5C.cpp+-file:legacy_pointer_safety%5C.cpp&patternType=standard&sm=1&groupBy=repo

Does this mean if i set my compiler to -std=c++14 for example, and will include "memory" i would not get the full c++ 14 support (with std::declare_reachable for example)?

Yes, that is correct. However, I think that was widely seen as a mis-feature, hence why it was never implemented and then removed. Like @philnik said, libc++'s stance here is that we're better off making it a compilation error for people who try to use it than let it compile, but do nothing good. That technically makes us non-conforming in C++14 mode, however in practice it's better to be non-conforming than conforming (in that specific aspect).

libcxx/docs/ReleaseNotes.rst

- The call of the functions ``std::atomic_(add|sub)(std::atomic<T*>*, ...)``
   compiler are unaffected by this change.
 
 - The functions ``std::allocator<T>::allocate`` and
   ``std::experimental::pmr::polymorphic_allocator<T>::allocate`` now throw
   an exception of type ``std::bad_array_new_length`` when the requested size
   exceeds the maximum supported size, as required by the C++ standard.
   Previously the type ``std::length_error`` was used.
 
+- The declarations of functions ``declare_reachable``, ``undeclare_reachable``, ``declare_no_pointers``,
+  ``undeclare_no_pointers``, and ``get_pointer_safety`` have been removed not only from C++2b but
+  from all modes. Their symbols are still provided by the dynamic library for the benefit of
+  existing compiled code. All of these functions have always behaved as no-ops.
+
 ABI Changes
 -----------
 
 - The C++17 variable templates ``is_error_code_enum_v`` and
   ``is_error_condition_enum_v`` are now of type ``bool`` instead of ``size_t``.
 
 Build System Changes
 --------------------

libcxx/include/CMakeLists.txt

set(files
   __memory/addressof.h
   __memory/allocation_guard.h
   __memory/allocator_arg_t.h
   __memory/allocator_traits.h
   __memory/allocator.h
   __memory/auto_ptr.h
   __memory/compressed_pair.h
   __memory/construct_at.h
-  __memory/pointer_safety.h
   __memory/pointer_traits.h
   __memory/raw_storage_iterator.h
   __memory/shared_ptr.h
   __memory/temporary_buffer.h
   __memory/uninitialized_algorithms.h
   __memory/unique_ptr.h
   __memory/uses_allocator.h
   __mutex_base

libcxx/include/__memory/pointer_safety.h

-// -*- C++ -*-
-//===----------------------------------------------------------------------===//
-//
-// 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
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef _LIBCPP___MEMORY_POINTER_SAFETY_H
-#define _LIBCPP___MEMORY_POINTER_SAFETY_H
-
-#include <__config>
-#include <cstddef>
-
-#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
-#pragma GCC system_header
-#endif
-
-_LIBCPP_BEGIN_NAMESPACE_STD
-
-#if !defined(_LIBCPP_CXX03_LANG)
-
-enum class pointer_safety : unsigned char {
-  relaxed,
-  preferred,
-  strict
-};
-
-inline _LIBCPP_INLINE_VISIBILITY
-pointer_safety get_pointer_safety() _NOEXCEPT {
-  return pointer_safety::relaxed;
-}
-
-_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
-
-#endif // _LIBCPP___MEMORY_POINTER_SAFETY_H

libcxx/include/memory

 // Hash support
 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 }; // 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>
 #include <__functional_base>
 #include <__memory/addressof.h>
 #include <__memory/allocation_guard.h>
 #include <__memory/allocator.h>
 #include <__memory/allocator_arg_t.h>
 #include <__memory/allocator_traits.h>
 #include <__memory/compressed_pair.h>
 #include <__memory/construct_at.h>
-#include <__memory/pointer_safety.h>
 #include <__memory/pointer_traits.h>
 #include <__memory/raw_storage_iterator.h>
 #include <__memory/shared_ptr.h>
 #include <__memory/temporary_buffer.h>
 #include <__memory/uninitialized_algorithms.h>
 #include <__memory/unique_ptr.h>
 #include <__memory/uses_allocator.h>
 #include <compare>

libcxx/include/module.modulemap

module __memory {
       module addressof                { private header "__memory/addressof.h"                }
       module allocation_guard         { private header "__memory/allocation_guard.h"         }
       module allocator                { private header "__memory/allocator.h"                }
       module allocator_arg_t          { private header "__memory/allocator_arg_t.h"          }
       module allocator_traits         { private header "__memory/allocator_traits.h"         }
       module auto_ptr                 { private header "__memory/auto_ptr.h"                 }
       module compressed_pair          { private header "__memory/compressed_pair.h"          }
       module construct_at             { private header "__memory/construct_at.h"             }
-      module pointer_safety           { private header "__memory/pointer_safety.h"           }
       module pointer_traits           { private header "__memory/pointer_traits.h"           }
       module raw_storage_iterator     { private header "__memory/raw_storage_iterator.h"     }
       module shared_ptr               { private header "__memory/shared_ptr.h"               }
       module temporary_buffer         { private header "__memory/temporary_buffer.h"         }
       module uninitialized_algorithms { private header "__memory/uninitialized_algorithms.h" }
       module unique_ptr               { private header "__memory/unique_ptr.h"               }
       module uses_allocator           { private header "__memory/uses_allocator.h"           }
     }

libcxx/src/CMakeLists.txt

set(LIBCXX_SOURCES
   exception.cpp
   functional.cpp
   future.cpp
   hash.cpp
   include/apple_availability.h
   include/atomic_support.h
   include/config_elast.h
   include/refstring.h
+  legacy_pointer_safety.cpp
   memory.cpp
   mutex.cpp
   mutex_destructor.cpp
   new.cpp
   optional.cpp
   random_shuffle.cpp
   shared_mutex.cpp
   stdexcept.cpp

libcxx/src/legacy_pointer_safety.cpp

+//===----------------- legacy_pointer_safety.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
+//
+//===----------------------------------------------------------------------===//
+
+#include "__config"
+#include <memory>
+
+// pointer_safety functions are removed in C++23 and are not declared anymore in any C++ version in libc++.
+// They are defined here for ABI backwards compatibility.
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+void declare_reachable(void*) {}
+void declare_no_pointers(char*, size_t) {}
+void undeclare_no_pointers(char*, size_t) {}
+void* __undeclare_reachable(void* p) { return p; }
+
+_LIBCPP_END_NAMESPACE_STD

libcxx/src/memory.cpp

static __sp_mut muts[__sp_mut_count]
         &mut_back[ 8], &mut_back[ 9], &mut_back[10], &mut_back[11],
         &mut_back[12], &mut_back[13], &mut_back[14], &mut_back[15]
     };
     return muts[hash<const void*>()(p) & (__sp_mut_count-1)];
 }
 
 #endif // !defined(_LIBCPP_HAS_NO_ATOMIC_HEADER)
 
-void
-declare_reachable(void*)
-{
-}
-
-void
-declare_no_pointers(char*, size_t)
-{
-}
-
-void
-undeclare_no_pointers(char*, size_t)
-{
-}
-
-void*
-__undeclare_reachable(void* p)
-{
-    return p;
-}
-
 void*
 align(size_t alignment, size_t size, void*& ptr, size_t& space)
 {
     void* r = nullptr;
     if (size <= space)
     {
         char* p1 = static_cast<char*>(ptr);
         char* p2 = reinterpret_cast<char*>(reinterpret_cast<size_t>(p1 + (alignment - 1)) & -alignment);

libcxx/test/libcxx/diagnostics/detail.headers/memory/pointer_safety.module.verify.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
-//
-//===----------------------------------------------------------------------===//
-
-// REQUIRES: modules-build
-
-// WARNING: This test was generated by 'generate_private_header_tests.py'
-// and should not be edited manually.
-
-// expected-error@*:* {{use of private header from outside its module: '__memory/pointer_safety.h'}}
-#include <__memory/pointer_safety.h>

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"
-
-int main(int, char**)
-{
-    char* p = new char[10];
-    std::declare_no_pointers(p, 10);
-    std::undeclare_no_pointers(p, 10);
-    delete [] p;
-
-  return 0;
-}

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>
-
-#include "test_macros.h"
-
-int main(int, char**)
-{
-    int* p = new int;
-    std::declare_reachable(p);
-    assert(std::undeclare_reachable(p) == p);
-    delete p;
-
-  return 0;
-}

libcxx/test/std/utilities/memory/util.dynamic.safety/get_pointer_safety.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();
-
-// UNSUPPORTED: c++03
-
-#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);
-  }
-  // Regression test for https://llvm.org/PR26961
-  {
-    std::pointer_safety d;
-    d = std::get_pointer_safety();
-    assert(d == std::get_pointer_safety());
-  }
-
-  return 0;
-}