This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] Make LIBCXX_HERMETIC_STATIC_LIBRARY apply to libc++experimental too
ClosedPublic

Authored by mstorsjo on Jul 7 2022, 3:33 AM.

Details

Summary

This avoids dllexports in that library.

Diff Detail

Event Timeline

mstorsjo created this revision.Jul 7 2022, 3:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 7 2022, 3:33 AM
Herald added a subscriber: mgorny. · View Herald Transcript
mstorsjo requested review of this revision.Jul 7 2022, 3:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 7 2022, 3:33 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne accepted this revision.Jul 7 2022, 8:00 AM
This revision is now accepted and ready to land.Jul 7 2022, 8:00 AM
ldionne added inline comments.Jul 7 2022, 8:01 AM
libcxx/src/CMakeLists.txt
337

Shouldn't we always add these flags to cxx_experimental, i.e. not only when LIBCXX_HERMETIC_STATIC_LIBRARY=ON?

mstorsjo added inline comments.Jul 7 2022, 8:37 AM
libcxx/src/CMakeLists.txt
337

Well this is how it’s done for the regular static library above, lines 295-302.

mstorsjo added inline comments.Jul 7 2022, 1:30 PM
libcxx/src/CMakeLists.txt
337

The only place CXX_STATIC_LIBRARY_FLAGS is set is on line 300 above, where -fvisibility-global-new-delete-hidden is added to the variable. But I guess that we don't need to set that at all for libc++experimental, as that library doesn't provide any new/delete operators?

I can rerun this patch in CI with the target_compile_options(cxx_experimental PRIVATE ${CXX_STATIC_LIBRARY_FLAGS}) line omitted, as it's not needed for the case I'm fixing here anyway.

mstorsjo updated this revision to Diff 443046.Jul 7 2022, 1:34 PM

Removed the setting of CXX_STATIC_LIBRARY_FLAGS for cxx_+experimental, as it probably isn't needed. Will push later when CI has passed.

phosek accepted this revision.Jul 7 2022, 2:57 PM

LGTM