This change is ABI breaking.
Previously reserve was explicitly instantiated in the dylib. After this change it is not.
I think we need to re-implement shrink_to_fit after this change, since it is currently implemented as reserve(0).
Since the new behavior is also allowed prior to C++20, I wonder if it would be better to make this change unconditionally?
One possibility would be to introduce a new define, for example _LIBCPP_INLINE_VISIBILITY_AFTER_CXX17. libc++.so is compiled in C++11 dialect so these symbols would be included for pre-C++17, while those setting dialect C++2a (or newer) would get the inline version.
But if we eliminate the #if altogether then this would go away.
Good catch with shrink_to_fit. I will fix that.
I was considering that as well.
My only concern was that older applications that may rely on using reserve to set the capacity (rather than using shrink_to_fit as is proper in C++20) might be affected by this change (behavior changes are more expected when you change the -std dialect options). I'm not too concerned about this, but I'm not familiar with libc++ development practices.
If the consensus is that this is fine, I can make that change. This also solves the issue with changing the ABI by making the function inline.
Libc++ may not be compiled in the C++11 dialect. We shouldn't depend on that being the case.
What I think we want to do is something like this:
#ifndef _LIBCPP_BUILDING_LIBRARY #define _LIBCPP_INLINE_VISIBILITY #endif void reserve(size_type __capacity_arg);
This needs an _LIBCPP_INLINE_VISIBILITY because we don't want this symbol going into the dylib. That would break backwards compatibility issues.
I think we should make this reserve call act the same as it did prior to C++2a. That is, it should call __request_capacity(0) or shink_to_fit()
After some more thinking, I think it's best we leave the old behavior as-is. All of the existing standard libraries seem to honor the shrink request prior to C++20, and that provides mostly consistent behavior to users. For consistency alone I think it's probably better not to backport this extension.
Made some changes to keep behavior the same in older -std= dialects. We shouldn't overload the function to avoid code such as:
auto f(&::std::string::reserve); // Ambiguous post C++17
from breaking for std < 17 (will break if -std=c++2a however)
Users are not allowed to take the address of standard library member functions.
See [member.functions]/2 for a description of why - the function you want to take an address of may not exist.
You are right, it is not guaranteed per the standard, although it has worked before (at least on MSVC). Unfortunately, I've seen many instances of code that relies on this implementation-specific behavior (particularly older code that uses bind instead of lambdas).
Anyways though, if you think we should go ahead and split the functions for all dialects anyways (C++2a as well as pre-C++2a) then I can revert that part of the changes...
For libstdc++ I think we have no real choice but to change it for all dialects. Our std::string::reserve(size_type) is exported from the shared library, so users will always get whatever behaviour the library exports, irrespective of their -std options. So we'll be changing reserve(size_type) to never shrink, unconditionally. So the consistency argument is lost.
Is it not possible to do the same as here in libstdc++:
Old version of reserve(size_type) is instantiated in the library (thus old code will still receive the old behavior) while marking the function inline for C++2a and forward (thus newer code will see the newer behavior)?