[PATCH] D56997: Fix implementation of P0966 - string::reserve Should Not Shrink

Eric Fiselier via Phabricator reviews at reviews.llvm.org
Sun Jan 20 20:55:08 PST 2019


EricWF added inline comments.


================
Comment at: include/string:963
 
-    void reserve(size_type __res_arg);
+    _LIBCPP_INLINE_VISIBILITY void reserve(size_type __res_arg);
     _LIBCPP_INLINE_VISIBILITY void __resize_default_init(size_type __n);
----------------
This change is ABI breaking.

Previously `reserve` was explicitly instantiated in the dylib. After this change it is not.
I'll think more about what the best way to handle this is.


================
Comment at: include/string:3147
     size_type __sz = size();
+#if _LIBCPP_STD_VER > 17
+    if (__res_arg <= __cap)
----------------
I think we need to re-implement `shrink_to_fit`  after this change, since it is currently implemented as `reserve(0)`. 


================
Comment at: include/string:3147
     size_type __sz = size();
+#if _LIBCPP_STD_VER > 17
+    if (__res_arg <= __cap)
----------------
EricWF wrote:
> 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?


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56997/new/

https://reviews.llvm.org/D56997





More information about the libcxx-commits mailing list