RFR: 8327045: Consolidate -fvisibility=hidden as a basic flag for all compilation

Magnus Ihse Bursie ihse at openjdk.org
Fri Mar 8 17:03:01 UTC 2024


On Thu, 29 Feb 2024 20:15:23 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

> WRT the change from `EXPORT` to `JNIEXPORT`, is that a required change, or stylistic?
> 
> From a style perspective, it would be preferable not to use `JNIEXPORT`, since these functions are not JNI functions. But also, I think we would like to avoid accidental dependencies on JNI in the java/foreign tests. These are meant to be 'plain' native libraries.

It is not strictly *required*, but the alternative is to define EXPORT exactly as JNIEXPORT is defined, for all platforms. In practice, `JNIEXPORT` has been used as the de-facto keyword to mark exported functions in the JDK project. One could argue about the name (a plain `EXPORT` or perhaps `JDK_EXPORT` would have been better imho), but this is how it has actually ended up. So while the name indicates "something to do with JNI", this is not at all the case.

If the amount of code churn is concerning, I could back out these changes and add a `#define EXPORT JNIEXPORT` in `shared.h`. (I was considering this solution initially, but thought it was better to explicitly use JNIEXPORT here as in the rest of the product.)

If you *really* insist on not including `jni.h`, I would need to duplicate all the platform-specific JNIEXPORT definitions from that file. I can do it (I understand your reluctance of risking to bring in unintentional stuff), but it will also cause code duplication that is a bit unfortunate.

The best solution would be if there was a globally recognized file available for both the JDK product and the tests that basically just defined `EXPORT` in a proper way, with no other bagage. I could try to introduce such a file, but doing system-wide changes like that is usually a months-long exercise in trying to get everyone to agree. I'd like to see such a file being a fact, but I do not like the prospect of trying to push through such a change...

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

PR Comment: https://git.openjdk.org/jdk/pull/18061#issuecomment-1973242641


More information about the build-dev mailing list