[libcxx-commits] [PATCH] D73743: Inline basic_string::erase for fastpath where __n == npos

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

You'll need to update the ABI lists for the unstable ABI to account for the addition of `__erase_external`. Try running `ninja check-cxx-abilist`.

Lurkers: Martijn and I spoke at length and we decided to try and have two fully separate lists of exported symbols for the stable and unstable ABIs. We think this will make it clearer what's exported in what configuration, etc. In particular, I didn't like the fact that in the stable ABI this patch started exporting `__erase_external` even though it would never actually be used from the dylib (cause otherwise it's an ABI break for back-deployment). Martijn nicely agreed to prototype a patch that splits the two lists out, and we'll see which way we go based on that patch.



================
Comment at: libcxx/include/string:3002
 
+// 'external instantiated' erase() implementation, called when __n != npos.
+// Does not check __pos against size()
----------------
'externally instantiated' ?


================
Comment at: libcxx/include/string:3024
 
+// Inline-able portion of the erase() implementation (unstable)
+template <class _CharT, class _Traits, class _Allocator>
----------------
In what way is this unstable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73743