On Wed, 29 Sep 2021 22:52:08 GMT, Jonathan Gibbons <[email protected]> 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
Foo foo;
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.
-------------
PR: https://git.openjdk.java.net/jdk/pull/5728
