RFR: 8355204: Consider simplifying type metadata stripping
Maurizio Cimadamore
mcimadamore at openjdk.org
Tue Jul 29 17:53:15 UTC 2025
On Mon, 21 Apr 2025 16:41:15 GMT, Liam Miller-Cushon <cushon at openjdk.org> wrote:
> This change removes support for `stripMetadataIfNeeded`. After [JDK-8355065](https://bugs.openjdk.org/browse/JDK-8355065) there are only two uses of the method:
>
> * `Types#directSupertypes`, which is specified as preserving type annotations. This change means that type metadata will be preserved for supertypes that appear in source, as well as substituted types for transitive supertypes.
> * `Trees#getTypeMirror`, which doesn't specify whether or not type annotations are preserved.
>
> `stripMetadataIfNeeded` was added for [JDK-8031744](https://bugs.openjdk.org/browse/JDK-8031744). The unconditional `stripMetadata` was added for [JDK-8144580](https://bugs.openjdk.org/browse/JDK-8144580). `stripMetadata` was substantially reworked to ensure it always stripped annotations in [JDK-8042981](https://bugs.openjdk.org/browse/JDK-8042981), which also updated the spec for Types' utility methods to clarify when type annotations were preserved, added a public API for removing annotations from types, and replaced some calls to `stripMetadataIfNeeded` with `stripMetadata` because the former didn't strip annotations in some cases where stripping was actually needed.
>
> I think it's worth considering removing `stripMetadataIfNeeded`. The bookkeeping to keep track of whether or not metadata should be stripped is fragile, and the API use-case can be supported by either preserving metadata or unconditionally stripping it.
The comment here:
https://github.com/openjdk/jdk/pull/8984#issuecomment-1166669173
Seems to indicate that a decision was made for `directSupertypes` to retain type annotations (presumably on the basis that such annotations are "declared"). I'm not too sure about that -- if we start from a `TypeElement` `E` and we want to know the `TypeMirror` corresponding to the direct supertype of the mirror associated with that `E`, then it seems ok for the mirror to contain type annotations: after all the mirror represents a type that was declared somewhere in the source code.
But if I want to compute the mirror that is the direct supertypes of `List<@NonNull String>` I'm not sure it's fair game for the supertype mirror to be `Collection<@NonNull String>`. The main point of `stripMetadataIfNeeded` was to strip type annotation whenever javac applied some transformation to a declared type. This was based on the principle that declared types should have their annotations reflected, whereas types _derived_ from declared types (e.g. via substitution) should not contain type annotations.
That said, the PR referenced above introduced new text to say that `Annotations on the direct supertypes are preserved.`. But then I see the code calling `stripMetadataIfNeeded`, so I'm not sure how to reconcile the implementation with the javadoc. @jddarcy can you please clarify what was the desired behavior for `Types::directSupertypes` ? The current implementation will _sometimes_ retain annotations, sometimes not, based on whether the supertype is a derived type or not (see above). If that is the intended behavior, then this PR that drops the conditional stripping will not be ok. But if the intention is for _all_ calls to `directSupertypes` to always retain type annotations, then this PR is ok.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/24775#issuecomment-3133482962
More information about the compiler-dev
mailing list