Logic cleanups for basic_string

[achabense]

(Updated)

1. Makes string(const T*) and string(const T*, const Alloc&) adjacent. They work as a whole to resolve to string(const T*, const Alloc&={}).
2. Rectify usages of small/large string mode. These changes will eliminate all direct comparisons with _BUF_SIZE in basic_string.

• Introduces _SMALL_STRING_CAPACITY, which equals _BUF_SIZE - 1, and replace every _BUF_SIZE - 1 in basic_string with it.
• Introduces _Entails_large_string(_Size/_Cap), which decides whether newly planned size/capacity requires large string mode.
• Refines _Large_string_engaged logic, and fixes a relevant misuse in operator=(const string&) (which will break invariant, and should be _Entails_large_string(_Mysize) instead).
• Introduces _LEAST_ALLOCATION_SIZE for _Move_assign_from_buffer and _Release_to_buffer.
• These changes will eliminate all direct comparisons with _BUF_SIZE in basic_string.

3. Rectify the relation between capacity and allocation size.

• Introduces _Allocate_for_capacity, which does stl check and lifetime activation, and takes capacity as input. This will eliminate all (al.allocate(res+1) ... _Start_element_lifetimes) pattern.
• Introduces _Deallocate_for_capacity counterpart.
• These two changes will remove all direct usages of al.deallocate, and only one wild al.allocate(without lifetime startup) is left in the interaction part with basic_stringbuf.

2 and 3 will make it easier to keep the following invariant:
capacity always== _SMALL_STRING_CAPACITY in small mode && capacity always> _SMALL_STRING_CAPACITY in large mode

[achabense]

I think dropping _Memcpy_val_from optimization (was in _Copy_assign_val_from_small) is ok here.
Otherwise we have to deal with three cases:
right is in small mode(res) and in non-constexpr context (and !defined(_INSERT_STRING_ANNOTATION)) -> can do _Memcpy_val_from
right is small(size) -> copy str without allocation
right is large(size) -> allocate and copy str

Hm... seems not hard to add the optimization back?

[achabense]

This part was broken. It is a misuse of _Large_string_engaged, and can cause allocation smaller than BUF_SIZE. (e.g. consider when large_string_engaged(), but size() == 0)

[achabense]

If this fix is adopted, the second parameter of _Calculate_growth(x,y,z) will be uniformly _SMALL_STRING_CAPACITY and further rectification will be possible.

There is also a special use of _Calculate_growth(newsize) in _Construct_from_iter. According to impl logic it is performing capacity*=1.5 logic. What about replacing the grow logic here with _Reallocate_grow_by or push_back?

STL/stl/inc/xstring

Line 2712 in 40640c6

const size_type _New_capacity = _Calculate_growth(_My_data._Mysize);

[achabense]

It was this chec that were causing lots of test failures, as

1. _Myres is(was) initialized to 0 on construction.
2. In several places(like _Construct), _Large_string_engaged is called early before assignment of _Myres.

After making _Myres initialized to _SMALL_STRING_CAPACITY, I made some checks passed, however there were some errors in clang checks due to constexpr evaluation hit maximum step limit.

Here I decide to remove this check to pass those clang tests. However, I'm uncertain wether this check is helpful. Does the check make sense? Should I add it back?

[achabense]

I'm leaving _Myres initialized to _SMALL_STRING_CAPACITY, as _String_val begins with SSO mode. This change will make some assignments redundant: (Candidates: (1) (2) (3); they are unchanged now)

[StephanTLavavej]

I am generally concerned about the magnitude of these changes; there are a lot of transformations happening here, and having them all in a single PR (without well-structured fine-grained commits) makes the validity of each transformation difficult to review. I think it's reviewable but I'll need some time.

[StephanTLavavej]

I think I can group up these changes. Do I need to reorganize commits and reupload this pr?

If you have time, that would be great! I started doing that, but I ran out of time today (and will need to work on a Patch Tuesday toolset update tomorrow before I can resume reviewing this PR). I pushed a conflict-free merge with main plus a couple of changes for issues that I identified; please pick up these changes (assuming you agree with them, of course!).

You can see main...StephanTLavavej:STL:fine-grained for an idea of what commits would be easiest to review. When each commit is focused on a single transformation whose correctness can be verified at each location, or is focused on a single fix, then they can be reviewed sequentially much easier without the risk of missing anything.

We usually discourage force-pushes for PRs that are under review, since GitHub doesn't make it easy to see what changed. In this case, because I've paused reviewing and the other maintainers are all busy, it's your call - you can either force-push this PR, or you can close this PR and open a new one. Either way, I recommend verifying that your commit restructuring isn't introducing unintentional changes - you can simply git diff OLD_BRANCH NEW_BRANCH to verify that they're ending up with the same content.

Thanks! 😻 Your changes look like a great refactoring, I just want to be really careful about reviewing it given how complicated string is, and how absolutely critical its correctness is.

[achabense]

Ok, I will do that. I'm going to close this pr and make a new one.