[libcxx-commits] [PATCH] D112869: [libc++] Implement P2186R2 (Remove Garbage Collection)

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone requested changes to this revision.
Quuxplusone added a comment.
This revision now requires changes to proceed.

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`.



================
Comment at: libcxx/include/__memory/pointer_safety.h:22
 
-#if !defined(_LIBCPP_CXX03_LANG)
+#if !defined(_LIBCPP_CXX03_LANG) && _LIBCPP_STD_VER <= 20
 
----------------
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).


================
Comment at: libcxx/include/__memory/pointer_safety.h:38
 _LIBCPP_FUNC_VIS void undeclare_no_pointers(char* __p, size_t __n);
 _LIBCPP_FUNC_VIS void* __undeclare_reachable(void* __p);
 
----------------
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.)



================
Comment at: libcxx/include/__memory/pointer_safety.h:48
 
-#endif // !C++03
+#endif // !C++03 && C++ <= 20
 
----------------
Pre-existing: This comment should just be
```
#endif // !defined(_LIBCPP_CXX03_LANG) && _LIBCPP_STD_VER <= 20
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112869/new/

https://reviews.llvm.org/D112869