[libcxx-commits] [PATCH] D91778: [libc++] [P0966] [C++20] Fix bug PR45368 by correctly implementing P0966: string::reserve should not shrink.

[Richard Smith - zygoloid via Phabricator via libcxx-commits]

rsmith added inline comments.


================
Comment at: libcxx/include/string:3273-3276
+#if _LIBCPP_STD_VER > 17
+    // Reserve never shrinks as of C++20.
+    if (__requested_capacity <= capacity()) return;
+#endif
----------------
rsmith wrote:
> ldionne wrote:
> > rsmith wrote:
> > > This `#if` doesn't seem to work: because `reserve(size_type)` is not inline, we always use the version from the libc++ library, which means we get the C++17 behavior if the library was built in C++17 mode, and we get the C++20 behavior if the library was built in C++20 mode, regardless of the standard mode used by the compilation using libc++.
> > Oh crap, that's right. Nice catch. But I'm not seeing a way to fix this except to make this function inline, and to keep a definition in the dylib for backwards compatibility (but nobody except already-compiled code would call it). Do you see other ways?
> If we could somehow split it into two functions for C++17 and C++20 calls, that might work.
> 
> In principle, if we could rely on concepts in C++20 mode, then I think that making the "never shrinks" C++20 overload be `requires true` + inline and leaving the C++17 version unconstrained and non-inline should do the trick with no ABI impact. But that doesn't compile in Clang and crashes GCC: https://godbolt.org/z/3WEKMa -- looks like neither compiler properly mangles constrained functions yet.
> 
> So how about something like this: https://godbolt.org/z/M69EP1
Looks like this was landed without fixing this problem. See https://github.com/llvm/llvm-project/issues/53170


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91778