[libc++] shrink_to_fit never shrinks

[winner245]

The current implementation of the shrink_to_fit function in vector<bool> effectively acts as a no-op, failing to shrink the capacity to fit the size. This can be demonstrated with the following example:

Godbolt Link

#include <iostream>
#include <vector>

int main() {
  std::vector<bool> v(1024, true);
  std::cout << "Before shrink: capacity = " << v.capacity() << '\n';

  v.erase(v.begin() + 512, v.end());
  v.shrink_to_fit();
  std::cout << "After shrink:  capacity = " << v.capacity() << '\n';

  v.erase(v.begin(), v.end());
  v.shrink_to_fit();
  std::cout << "After shrink:  capacity = " << v.capacity() << '\n';
}

Program output:

Before shrink: capacity = 1024
After shrink:  capacity = 1024
After shrink:  capacity = 1024

This is because the current implementation of shrink_to_fit checks the following condition

if (__external_cap_to_internal(size()) > __cap_) {
  try {
    do_shrink();
  } catch (...) {
  }  
}

However, this condition is always false, as the number of used internal words __external_cap_to_internal(size()) will never exceed the internal storage capacity __cap_. Thus, the above implementation is equivalent to

if (false) {
  ...
}

which will never shrink.

(Note: Previously, there was an exceptional scenario where the evaluation of __external_cap_to_internal(0) led to size_type(-1) wrapping around to the maximum unsigned value, causing this condition to be true. However, this evaluation was incorrect and has been fixed in #120577.)

While the current implementation of shrink_to_fit technically conforms to the standard—since the shrink-to-fit request is non-binding—it appears to be a logical error that messed up > and <, particularly since a similar logical error was found in the __split_buffer::reserve, where < was incorrectly used instead of > (an issue reported in #105681 and fixed in #115735).

Proposed Solution

If the intent is to keep the existing no-op behavior, the function should be modified to have an empty body, possibly accompanied by a comment for clarity. The current implementation, which is non-empty yet effectively behaves as empty, is misleading.

Another approach would be modifying shrink_to_fit to actually perform capacity reduction, aligning its behavior with the shrink_to_fit function of std::vector<T>. This approach is currently being pursued in #120495.

[ldionne]

This is clearly a bug, we should be shrinking the vector. It's a QOI matter even if that doesn't make us non-conforming.