RFR: 8274405: Suppress warnings on non-serializable non-transient instance fields in javac and javadoc [v3]

Jonathan Gibbons jjg at openjdk.java.net
Wed Sep 29 22:55:38 UTC 2021

On Wed, 29 Sep 2021 22:03:00 GMT, Joe Darcy <darcy at openjdk.org> wrote:

>> Augmentations to javac's Xlint:serial checking are out for review (#5709) and various javac and javadoc implementation libraries would need some changes to pass under the expanded checks.
>> The changes are to suppress warnings where non-transient fields in serializable types are not declared with a type statically known to be serializable. That isn't necessarily a correctness issues, but it does merit further scrutiny.
>> I'll run a script to update the copyright year before pushing.
>> Sending this change out for review separately in an effort to de-bulk the review needed for the new checks themselves.
> Joe Darcy has updated the pull request incrementally with one additional commit since the last revision:
>   Respond to review feedback.

There are 3, maybe just 2,  groups of files in this review.

`sjavac` is an internal utility that ought not to be in the `src/` tree, so the changes there don't matter.

The various `Result` classes and the javadoc exceptions are all "internal" exceptions used for internal control flow, and not intended to be seen by API clients.  As @pavelrappo notes, we have plans to make the `Result` classes go away by rewriting the relevant scanners to make them unnecessary. That leaves the javadoc exceptions. The commented annotations have a slight code-smell about them, in the sense of brushing the warning under the carpet, so to speak. It would be better if there was a better way to remove the cause of the warning, rather than just hiding the warning itself.  But, that's not easy, and the original sin was making `Throwable` (and hence all subtypes) implement `Serializable` in the first place. But, that's the serialization we have and we have to deal with it.

As a final note, I never did like the `Runnable` in the last exception, and seeing it again may be a good reason to try and get rid of it, like those `Result` classes mentioned earlier.   I also note that `DocPath` _could_ be made `Serializable` but `DocFile` cannot reasonably be made serializable (incompatible API change to `Location`) and even thinking about it seems like a case of the tail wagging the elephant!

So, with some disappointment, I accept that the changes you propose are the least bad of the possible solutions, at least for now, and so I approve the review.


Marked as reviewed by jjg (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/5728

More information about the compiler-dev mailing list