Make sure that our multi-release jar actually lets people use module-scope @NullMarked

[cpovirk]

I had found that it works for plain javac invocations, and I believe eamonnmcmanus was the one who confirmed for us that it works with NetBeans. But we had seen a report that it doesn't work with IntelliJ, and we haven't tried Eclipse.

And now... I tried to use module-scope @NullMarked in a Gradle project that uses --release 11, and:

error: annotation type not applicable to this kind of declaration

That's happening even though I see a jspecify-0.2.0.jar (with what looks like the right contents) on the --module-path when I pass -d to Gradle.

[edit: Apparently kotlinc doesn't look under META-INF/versions, either. But that's mostly academic: Users won't need to write JSpecify annotations in Kotlin sources (except in rare cases until KT-47417 is implemented), and even if they do, they won't be putting an annotation on a module.]

[edit: I wonder what Turbine does....]

[cpovirk]

On the Gradle front, I found a bug that sound potentially related: gradle/gradle#19587. I also wasn't surprised to find that the Gradle people recommend that library authors instead use their mechanism for separate jars, the thing that I continually feel guilty for not enabling for Guava. Separate jars is certainly a possible fallback option, though Guava users are familiar with some of the resulting pain, and Gradle's work to eliminate that pain won't help users of other build systems.

Also, to be clear: @KengoTODA had added some tests to make sure that the right version of @NullMarked is selected at runtime. The problem I was seeing was at compile time.

And that problem was:

$ ./gradlew compileJava
...
/usr/local/google/home/cpovirk/clients/caffeine-black/caffeine/caffeine/src/main/java/module-info.java:1: error: annotation type not applicable to this kind of declaration
@org.jspecify.nullness.NullMarked
^
1 error
...

...when I tried to make this change to Caffeine.

[Sineaggi]

At least in a small repo that doesn't have much code, adding the @NullMarked annotation to the module works for me. Intellij doesn't like it, but that doesn't prevent me from compile and verifying via gradle run

System.out.println(Arrays.toString(Main.class.getModule().getAnnotations()));

that

[@org.jspecify.nullness.NullMarked()]

is printed as output.

[Sineaggi]

Also I grabbed the source (and generated sources) from caffeine, pulled them into a new project and was able to compile with the NullMarked annotation. Not sure why the upstream caffeine project can't.

[cpovirk]

Thank you very much for checking that!

The Caffeine finding is still a little concerning: Whatever is going on with the Gradle configuration, clearly that particular compiler invocation is targeting Java 9 or higher, since it's compiling a module-info.

But it's great to know that a more vanilla configuration works: The problem isn't quite so bad, and we have an idea of where we could start looking for the problem.

[Sineaggi]

Perhaps there's a bug in javac? Or in how gradle invokes javac? I was able to get your branch to build by removing the
java.toolchain.languageVersion setting, which in turn seems to have replaced --release 11 with -target 11 -source 11.

[cpovirk]

Ah, that's very interesting! I had been thinking of "targeting Java 9 or higher" as a single yes/no question. But in fact, --release 11 means not only -target 11 -source 11 (which is enough to permit compiling a module-info) but also something like -bootclasspath java11 -mrjarversiontoread 11. (Not that -mrjarversiontoread is a real flag, and not that "bootclasspath" is really what we mean nowadays for system modules(???).) That suggests a problem somewhere outside javac, whether in Gradle or in some Gradle extension or in this particular build script.

[cpovirk]

(Sorry, my post is a bit cryptic -- trying to post quickly at the end of my day.)

[Sineaggi]

Fwiw, in my personal mini caffiene testing project, it's about as minimal as it gets, and I can reproduce the issue just by uncommenting the options.release setting in https://github.com/Sineaggi/caffeine-jspecify-testing/blob/main/build.gradle.kts

[Sineaggi]

whether in Gradle or in some Gradle extension or in this particular build script

Which might suggest it's not specific to caffiene's build script, but potentially in gradle itself.

[cpovirk]

Ouch! At minimum, it is quite surprising that release seemingly does not set the --release flag, at least in some cases—especially given the docs here (found via gradle/gradle#2510). Maybe I'm reading too much into your results, but it certainly looks that way at first glance.

To be fair:

• It's still not clear that mrjars are meant to support different @Target values at all.
• One of the motivations for using an mrjar was Android, and that should no longer be an issue.
• Even old versions of javac and --release 8 might not be an issue, though I'm less confident in my testing of that.

So, to recap, we're now seeing trouble in IntelliJ and sometimes Gradle, and I'm not sure we're getting anything out of it.

We may well still need our multi-compilation setup so that we can build Java 8 bytecode for our main classes while still building a module-info (which requires targeting Java 9+). But our attempt to have separate versions of NullMarked may be best reverted.

[cpovirk]

• Even old versions of javac and --release 8 might not be an issue, though I'm less confident in my testing of that.

No, wait, we still get the warning, at least in the testing I'm doing right now:

warning: unknown enum constant ElementType.MODULE

We may still have to live with that, given the alternative....

[cpovirk]

I put together #246 to include MODULE unconditionally. That can lead to other issues, but my theory is that the result will be less bad on net than the current situation. Or at least I'd like for our next release to provide people the opportunity to test that theory.

If Java 8 users do have a problem with the artifact we release after #246, they may at least have the option of defining an annotation that @Implies the normal @NullMarked annotation but omits MODULE from its @Target. (Perhaps we'd even supply a standard such annotation ourselves.) But I should say something about @Implies and JDK-8247797 somewhere.... [edit: #218 (comment)]

[ExE-Boss]

I looked into javac code, and the current behaviour for unknown ElementType values is to treat the annotation as applicable in all possible locations¹.

Similarly, an unknown enum constant in an annotation array value causes the whole array to be invalid²³ when viewed using reflection.

Footnotes

1. https://github.com/openjdk/jdk/blob/6586e5ae37e09a6d47f07758e710e1327e1c3be9/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java#L3438-L3439 ↩

2. https://github.com/openjdk/jdk/blob/6586e5ae37e09a6d47f07758e710e1327e1c3be9/src/java.base/share/classes/sun/reflect/annotation/AnnotationParser.java#L721-L727 ↩

3. https://github.com/openjdk/jdk/blob/6586e5ae37e09a6d47f07758e710e1327e1c3be9/src/java.base/share/classes/sun/reflect/annotation/AnnotationParser.java#L478-L483 ↩

[cpovirk]

Thanks.

javac's ElementType behavior apparently has some history behind it (JDK-8261610). We probably don't want to omit @Target from our annotation, but it's among our options, all of which have downsides :(

And the reflection source code suggests that the problem you likely already saw in https://github.com/jspecify/jspecify/pull/246/files#diff-6a22a6ede57062df5613483ebcc3fd278316078440bc35c7bf253ef49e89ee12R58 is still around even today.

[cpovirk]

I was just tempted to summarize our findings here as "the idea seems to be that a compiler should be able to look at whichever version it wants." I probably still will do that, but I want to note a small caveat: We seem to be able to put module-info under the mrjar root (#484) and still rely on tools to see it. I do wonder if that will actually continue to be the case as we try more tools....