_LIBCPP_ENABLE_CXX20_REMOVED_ALLOCATOR_MEMBERS breaks container constexpr-ness

[jyknight]

As mentioned in #69466, if you set the _LIBCPP_ENABLE_CXX20_REMOVED_ALLOCATOR_MEMBERS define, the restored allocator members are not constexpr, which leads to the vector and string types being non-constexpr too, because they find the deprecated allocator members, instead of going through allocator_traits as intended.

This is an extremely surprising result.

Other _LIBCPP_ENABLE_*_REMOVED_* flags do not change the behavior of normal conforming C++17/20 code, but just restore standalone functions/types, not affecting code which doesn't use them. I think that's generally a useful option to provide users of libc++, and would argue against removing most ENABLE_CXXNN_REMOVED options. (And, further, argue for adding more for future standards).

However, if the option breaks required semantics or is tricky to implement without the risk of breaking required semantics, then we should not provide it. Allocators/allocator_traits are certainly a very tricky area in that regard. Thus, I propose that libc++ should remove both the option _LIBCPP_ENABLE_CXX20_REMOVED_ALLOCATOR_MEMBERS and _LIBCPP_ENABLE_CXX20_REMOVED_ALLOCATOR_VOID_SPECIALIZATION.

Sadly, Google's codebase is currently dependent upon this broken mode, and others may have similarly enabled it without realizing the negative impact. Thus, I propose to not immediately remove the options, but to phase it in:

1. Now: announce the pending removal, stop implying them from the _LIBCPP_ENABLE_CXX20_REMOVED_FEATURES define, and update the documentation accordingly.
2. Approx February 2024 (after LLVM 18 release branch): remove support. [edited per comments below]

@ilya-biryukov @ldionne

[ldionne]

1. Now: announce the pending removal, stop implying them from the _LIBCPP_ENABLE_CXX20_REMOVED_FEATURES define, and update the documentation accordingly.
2. In early January: remove support (before next LLVM release branch).

I couldn't agree more! The only difference here is that I would still ship these macros in LLVM 18, and I would remove them in LLVM 19 instead. This provides a grace period of one release for people who are not using libc++ from trunk and are using the official LLVM releases instead (which should be most people).

[philnik777]

I think I also agree here. Not going through allocator_traits has always been a bad idea, and fixing this should be fairly trivial. Combining that with having a mode that results in very surprising behaviour seems like a pretty bad idea in hind sight.

[ilya-biryukov]

I also agree that removing the option is better than what we have today.

However, we need to plan for this cleanup. It is a little more involved for Google than just replacing with allocator_traits because when vector becomes constexpr more issues in the codebase are surfaced (mostly use of incomplete types inside vector template arguments with newly-constexpr methods). I have also observed a few compiler crashes after removing this option.

So longer timeline seems preferable, @ldionne's proposal to keep it in LLVM 18 and in LLVM 19 gives us that.