Nullable field nondeterministically has null check inserted anyhow
kennknowles opened this issue · 3 comments
Summary of issue: A field that is nullable is nondeterministically having a null check in the generated code, depending on what other build tasks occurred.
See https://lists.apache.org/thread/cdw4y50r3hl37z8y7x470m2mddclkqgv (from here you can gather more info from the thread)
Inline, paraphrasing the thread...
To reproduce:
git clone https://github.com/apache/beam
cd beam
git checkout 4ffeae4d2b800f2df36d2ea2eab549f2204d5691~1
./gradlew :runners:direct-java:compileJava
less
./runners/direct-java/build/generated/sources/annotationProcessor/java/main/org/apache/beam/runners/direct/AutoValue_ImmutableListBundleFactory_CommittedImmutableListBundle.java
git checkout 4ffeae4d2b800f2df36d2ea2eab549f2204d5691
./gradlew :runners:direct-java:compileJava
less
./runners/direct-java/build/generated/sources/annotationProcessor/java/main/org/apache/beam/runners/direct/AutoValue_ImmutableListBundleFactory_CommittedImmutableListBundle.java
./gradlew :runners:direct-java:compileJava --rerun-tasks
less
./runners/direct-java/build/generated/sources/annotationProcessor/java/main/org/apache/beam/runners/direct/AutoValue_ImmutableListBundleFactory_CommittedImmutableListBundle.java
abstract static class CommittedImmutableListBundle<T> implements CommittedBundle<T> {
public static <T> CommittedImmutableListBundle<T> create(
@Nullable PCollection<T> pcollection,
StructuralKey<?> key,
Iterable<WindowedValue<T>> committedElements,
Instant minElementTimestamp,
Instant synchronizedCompletionTime) {
return new AutoValue_ImmutableListBundleFactory_CommittedImmutableListBundle<>(
pcollection, key, committedElements, minElementTimestamp, synchronizedCompletionTime);
}
...
}
interface CommittedBundle<T> extends Bundle<T, PCollection<T>> {
/** Returns the PCollection that the elements of this bundle belong to. */
@Override
@Nullable
PCollection<T> getPCollection();
...
}
In all cases the PCollection
field should be nullable.
In a "good" build, the AutoValue generated code constructor does not check for null
AutoValue_ImmutableListBundleFactory_CommittedImmutableListBundle(
@Nullable PCollection<T> PCollection,
StructuralKey<?> key,
Iterable<WindowedValue<T>> elements,
Instant minimumTimestamp,
Instant synchronizedProcessingOutputWatermark) {
this.PCollection = PCollection;
in a 'bad' build, the autovalue java is re-generated, but has an added nullness check:
AutoValue_ImmutableListBundleFactory_CommittedImmutableListBundle(
PCollection<T> PCollection,
StructuralKey<?> key,
Iterable<WindowedValue<T>> elements,
Instant minimumTimestamp,
Instant synchronizedProcessingOutputWatermark) {
if (PCollection == null) {
throw new NullPointerException("Null PCollection");
}
this.PCollection = PCollection;
and then with --rerun-tasks
it gets re-re-generated without the nullness check.
The Java compiler has historically had a number of bugs with type-use annotations, like org.checkerframework.checker.nullness.qual.Nullable
. Specifically, it reports them correctly to annotation processors like AutoValue if the code in question is being compiled at the same time, but it doesn't always report them correctly if they are in already-compiled code. To see whether this is indeed the problem, you could try using javax.annotation.Nullable
instead. That's not a type-use annotation, and I see you already have javax.annotation.Nonnull
in the same source file so presumably this should work.
If this is indeed the problem, I'm afraid I don't have a good solution other than that.
@cushon may have further input.
I agree this sounds like something that could happen if the incremental compilation was sometimes compiling both classes as source, and sometimes compiling just one of them and reading the other as a .class
file. If that's what's happening, it's an interaction between gradle's incremental compilation behaviour and this javac issue: https://bugs.openjdk.org/browse/JDK-8225377
Oh interesting! Seems likely