No point in pretending that these methods are hidden - they are
actually exported from libc++.so. Extern template declarations make
them part of libc++ ABI.
This patch does not change libc++.so export list (at least on Linux).
Differential D14409
Remove visibility attributes from out-of-class method definitions in iostreams. ClosedPublic Authored by eugenis on Nov 5 2015, 6:02 PM.
Details
Summary No point in pretending that these methods are hidden - they are This patch does not change libc++.so export list (at least on Linux).
Diff Detail
Event TimelineComment Actions <string> is an interesting case. Setting hidden visibility on an "extern template" method is probably a bad practice. By itself, it would break everything: extern template promises that this method is available externally, and hidden visibility kinda invalidates this promise. But is libc++ it is always accompanied by another attribute which sidesteps the issue: always_inline (if inlining happends) and internal_linkage (in all cases) emit method body in the caller's translation unit. So, nothing would break if we leave <string> unchanged. Removing the attributes would add 254 exports to libc++ and not really achieve anything. Well, it could help with the link failure on compilers that do not support internal_linkage AND not always inline always_inline - are there any like this? Comment Actions Does the inline keyword have any effect when it's on function definitions that are externally instantiated? Comment Actions
I could not detect any difference in behavior with or without inline keyword. EricWF edited edge metadata. Comment ActionsLGTM. I built the library before this patch and then diffed the symbols after this change. No symbols have been removed or added. LGTM. This revision is now accepted and ready to land.Dec 9 2015, 3:06 PM Comment Actions
Actually, remove the inline breaks tests, because now the method is declared hidden (in-class), so template instantiation in libc++.so produces a hidden symbol. As an alternative, we could remove both "inline" and the in-class hidden attribute. Comment Actions
So, we can not remove "inline" because without it always_inline does not seem to have any effect. I'll land this change as is, if you don't mind. Comment Actions
That change would be an ABI break. This change is perfect as-is.
Revision Contents
Diff 39750 include/istream
include/ostream
include/sstream
include/streambuf
|