On Thu, 22 Feb 2024 13:52:26 GMT, Magnus Ihse Bursie <i...@openjdk.org> wrote:

>> Oh no, by fix I meant removing the HIDDEN attribute and somehow fixing that 
>> issue differently, not replacing everything with C++ Attributes!
>
> Well, I hope that can be done, but it depends... As I see it, there are three 
> possible ways to get rid of this `HIDDEN`.
> 
> 1) Accept that we export these functions on gcc, and just remove the 
> attribute. (Kind of capitulating to the bug.)
> 
> 2) See if there is a way we can work around this problem, by changing 
> something in how these functions are defined, that does not trigger this bug.
> 
> 3) Have this bug fixed in gcc, and move to a minimum version of gcc that 
> includes this fix.
> 
> I'm not terribly keen on any of these solutions. I think 2, if it is 
> possible, would be the simplest and cleanest. Or 1, if we really think this 
> HIDDEN workaround is worse than the problem it solves (unintentionally 
> exporting symbols).

I see, seems like it's an unfortunate situation where a fix is hard or even 
impossible. If we file a gcc bug for it now, it'll get fixed in some insanely 
new gcc version (such as gcc 15 or 16) and we'd have to jump to that version, 
which I'm sure no one is going to like :(

Nevertheless, since there are so many instances of it, I suggest #pragma GCC 
visibility push(hidden) instead of adding a new macro to this file, in my 
opinion it's somewhat cleaner and also makes the change smaller.

#pragma GCC visibility push(hidden)
// Junk that needs hidden visibility to work properly
#pragma GCC visibility pop

Also, why TARGET_COMPILER_gcc instead of __GNUC__? Does this have to work with 
Apple Clang on macOS as well?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/17955#discussion_r1499309573

Reply via email to