On Thu, 22 Feb 2024 09:55:21 GMT, Julian Waters <jwat...@openjdk.org> wrote:
>> We're currently using __attribute__ ((visibility ("X"))) elsewhere, so I
>> think I should stick with that.
>>
>> And, as I said in my long explanation, this is definitely a hack. The reason
>> it is here is that I want to make sure the symbol table is 100% identical
>> before and after this patch.
>>
>> Why this is needed, I don't know. I believe it is a bug in GCC, since we
>> compile with -fvisibility=hidden. But I don't really know. Maybe there is
>> something in the C++ standard that makes these functions behave differently.
>> Perhaps they are even slightly incorrectly defined?
>
> Ah, I should have clarified that I did read the explaination, just that it
> slipped my mind at the time. I can't remember if C++14 or C++17 is the
> version for compilers ignoring unknown attributes though
>
> Also, you can escape the underscores with \ to avoid GitHub formatting.
> GitHub can get annoying with its formatting sometimes
>
> Is there a chance this can be fixed in a followup somehow? I could help with
> that, given I have access to WSL2 for Linux
You mean making a search/replace from `__attribute__ ((foo))` to
`[[gnu::foo)]]` in the code base? It seems a bit like unnecessary churn to me.
We have 100+ places in the code with `__attribute__`. If you really want to try
to push that agenda, be my guest. But do it in a separate PR. ;-)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17955#discussion_r1499001259
