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