[libcxx-commits] [libcxx] [libc++] Fix the long/short flag position of basic_string for a narrow size_type (PR #236867)

[Hubert Tong via libcxx-commits]

hubert-reinterpretcast wrote:

> This patch fixes `basic_string` for allocators with a `size_type` narrower than 4 bytes under `_LIBCPP_ABI_ALTERNATE_STRING_LAYOUT`.

As noted in https://github.com/llvm/llvm-project/pull/234500#discussion_r4408132919 (and actually addressed by the proposed change), the problem exists for `size_type` of 4 bytes or more under certain conditions. Please fix the description and the PR title.

Additionally, for the cases where the `size_type` is less than 4 bytes: I don't think halving the maximum capacity (as occurs with the alternate layout for little-endian platforms) is a good idea if the maximum capacity is small to begin with. Since we know (from the aforementioned PR) that the alternate layout did not really work for those `size_type`s in previous LLVM releases, we have a window of opportunity to revisit the layout while it is still sufficiently broken that adoption is constrained (i.e., before this PR lands).

The halving could be avoided by storing half the capacity as is done for `__endian_factor == 2` cases, but I presume the `__endian_factor` is chosen such that `capacity()` does not need a shift unless one is needed to move a flag out of the least significant bit, and I would like to keep such avoidance.

@ldionne @philnik777, in the case where `__long` would have padding anyway, is there some appetite to leave the capacity as a plain `size_type` member (as opposed to a bitfield) and to use a separate byte for the "is long" flag? This approach provides an improvement where possible to do so without additional storage and avoids both the halving and the need for a new shift.

https://github.com/llvm/llvm-project/pull/236867