[libcxx-commits] [PATCH] D73807: [libc++] Optimize std::string's external instantiations in an ABI compatible way

[Martijn Vels via Phabricator via libcxx-commits]

mvels requested changes to this revision.
mvels added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/__string:79
   _Func(_LIBCPP_FUNC_VIS void basic_string<_CharType>::__init(value_type const*, size_type, size_type)) \
-  _Func(_LIBCPP_FUNC_VIS basic_string<_CharType>::basic_string(basic_string const&)) \
   _Func(_LIBCPP_FUNC_VIS basic_string<_CharType>& basic_string<_CharType>::replace(size_type, size_type, value_type const*)) \
----------------
We should probably not outline this ctor before we do the 'init_long_external'


================
Comment at: libcxx/src/string.cpp:21
 
+
 template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS __basic_string_common<true>;
----------------
nit : empty line added


================
Comment at: libcxx/src/string.cpp:27
 
+#define _LIBCPP_STRING_EXTERN_TEMPLATE_LEGACY_DEFINITION_LIST(_Func, _CharType) \
+  _Func(_LIBCPP_FUNC_VIS basic_string<_CharType>::basic_string(basic_string const&)) \
----------------
Should we add a comment that these entries should never be removed, ever? This for reviewers less familiar with ABI subtleties.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D73807