RFR: 8274405: Suppress warnings on non-serializable non-transient instance fields in javac and javadoc [v3]
darcy at openjdk.java.net
Wed Sep 29 23:33:46 UTC 2021
On Wed, 29 Sep 2021 22:52:08 GMT, Jonathan Gibbons <jjg at openjdk.org> wrote:
> 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.
If I were to see
@SuppressWarning("serial") // Type of field not Serializable
in new code I would question the serial-design of the class; that is one of my goals with augmenting the Xlint:serial checks, notice possibly questionable serial coding patterns sooner.
For types where maintaining cross-release serial compatibility is not strictly needed, one approach would be to mark the fields as transient and give the class a different serialVersionUID. If serial compatibility is needed the conceptually transient field, i.e. one that cannot meaningful be serialized, could be nulled-out in a writeObject method and ignored in a readObject method, at the cost of maintaining those additional methods.
Fortunately, the the javax.lang.model API, the issues with exception classes having non-Serializable fields was recognized during the design and we marked all such fields and transient and documented the corresponding getters to possibly return null.
More information about the compiler-dev