[libcxx-commits] [PATCH] D150896: [libc++] Apply _LIBCPP_EXCLUDE_FROM_EXPLICIT_INSTANTIATION only in classes that we have instantiated externally

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik created this revision.
Herald added subscribers: mstorsjo, arichardson.
Herald added a project: All.
philnik updated this revision to Diff 523572.
philnik added a comment.
philnik updated this revision to Diff 523805.
philnik updated this revision to Diff 523827.
philnik updated this revision to Diff 523884.
philnik updated this revision to Diff 523960.
philnik updated this revision to Diff 524114.
philnik updated this revision to Diff 524383.
philnik marked 3 inline comments as done.
philnik published this revision for review.
Herald added a project: libc++.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libc++.

Try to fix CI


philnik added a comment.

Try to fix CI


philnik added a comment.

Try to fix CI


philnik added a comment.

Try to fix CI


philnik added a comment.

Try to fix CI


philnik added a comment.

Try to fix CI


philnik added a comment.

Address comments



================
Comment at: libcxx/include/__config:621-622
 //
 // Note that we use _LIBCPP_EXCLUDE_FROM_EXPLICIT_INSTANTIATION to ensure that we don't depend
 // on _LIBCPP_HIDE_FROM_ABI methods of classes explicitly instantiated in the dynamic library.
 //
----------------
This needs to be updated.


================
Comment at: libcxx/include/__config:648-656
 #  ifdef _LIBCPP_BUILDING_LIBRARY
 #    if _LIBCPP_ABI_VERSION > 1
-#      define _LIBCPP_HIDE_FROM_ABI_AFTER_V1 _LIBCPP_HIDE_FROM_ABI
+#      define _LIBCPP_HIDE_FROM_ABI_AFTER_V1 _LIBCPP_HIDE_FROM_ABI _LIBCPP_EXCLUDE_FROM_EXPLICIT_INSTANTIATION
 #    else
 #      define _LIBCPP_HIDE_FROM_ABI_AFTER_V1
 #    endif
 #  else
----------------
Let's add a comment explaining that `_LIBCPP_HIDE_FROM_ABI_AFTER_V1` also prevents newly compiled programs from relying on symbols that we know are removed from the ABI in a future version.


================
Comment at: libcxx/include/__locale:207
 
 template <class _CharT>
 class _LIBCPP_TEMPLATE_VIS collate
----------------
Could you please make sure that this patch doesn't change the set of symbols instantiated in the dylib (including hidden symbols)? If the dylib contains a new symbol X after this change, it would indicate that you forgot to add `_LIBCPP_EXCLUDE_FROM_EXPLICIT_INSTANTIATION` to symbol X in this patch. We should be careful to try to catch this here cause this is going to be a linker error potentially far downstream otherwise, in case we have missing coverage for symbol X in our test suite.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150896

Files:
  libcxx/include/__config
  libcxx/include/__locale
  libcxx/include/fstream
  libcxx/include/ios
  libcxx/include/istream
  libcxx/include/locale
  libcxx/include/ostream
  libcxx/include/regex
  libcxx/include/sstream
  libcxx/include/streambuf
  libcxx/utils/libcxx/test/params.py

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D150896.524383.patch
Type: text/x-patch
Size: 57822 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20230522/aea1a1c2/attachment-0001.bin>