libcxx: std::ostream::sentry should be exported #140169
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jeremyd2019
changed the title
jeremyd2019
force-pushed
the
libcxx-inner-class-export
branch
3 times, most recently
from
41904da to
56b71d9
Compare
… template should be instantiated In-code comment says "explicit instantiation decl of the outer class doesn't affect the inner class" but this behavior seems MSVC specific, MinGW-GCC and Cygwin-GCC does not. Clang should honor gcc's behavior. This change fixes std::string compatibilty and resolves strange link error (statically linked), strange crash (dynamically linked) using libstdc++ on Cygwin.
jeremyd2019
force-pushed
the
libcxx-inner-class-export
branch
from
56b71d9 to
73c338c
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
FYI, note that even if you include clang changes here, those won't be used by the libcxx CI build; the CI uses a prebuilt build of llvm-mingw - see |
|
I know, I just started with #140145 for my testing, this pr is really meant for testing that no other platform is broken. https://github.com/jeremyd2019/llvm-mingw/actions/runs/15075388808 is running the full test of af82a52 and now https://github.com/jeremyd2019/llvm-mingw/actions/runs/15078849158 for 04ef889 (since at least i686 already failed due to istream which was not done as a test to make sure the issue was reproducing) |
jeremyd2019
force-pushed
the
libcxx-inner-class-export
branch
2 times, most recently
from
233fc83 to
8ca8bdc
Compare
jeremyd2019
force-pushed
the
libcxx-inner-class-export
branch
4 times, most recently
from
2dca920 to
ce1573e
Compare
jeremyd2019
force-pushed
the
libcxx-inner-class-export
branch
from
ce1573e to
5035da0
Compare
use test llvm-mingw toolchain
…bility with Mingw-GCC In MinGW environment, Clang handles dllexport attribute of internal class that defined in class template in different way from GCC. This incompatibility should be fixed but breaks ABI of libc++, so introduce a new keyword to keep ABI in MinGW environment with old and patched Clang and to stay ABI compatible on other platforms. This attribute is attached only for basic_ostream::sentry::sentry, basic_ostream::sentry::~sentry and basic_istream::sentry::sentry. Other entities won't be affected by patching Clang so doesn't need to be annotate. At a time to introduce a new class as a non-template inner type of a class template, all non-inline member functions of that class also needs to be attached _LIBCPP_HIDE_FROM_ABI_MINGW_OR_AFTER_V1 or _LIBCPP_HIDE_FROM_ABI, not _LIBCPP_HIDE_FROM_ABI_AFTER_V1. Otherwise, that member functions contained in DLL will be inaccessible on MinGW environment. For time to increase ABI version, _LIBCPP_HIDE_FROM_ABI_MINGW_OR_AFTER_V1 can be simply replaced to _LIBCPP_HIDE_FROM_ABI_AFTER_V1.
jeremyd2019
force-pushed
the
libcxx-inner-class-export
branch
from
5035da0 to
54221fd
Compare
Since it seems so hard to get to stages 2 and 3
jeremyd2019
force-pushed
the
libcxx-inner-class-export
branch
from
5d1e523 to
bffb58e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Not for review. Testing another idea for #140145