RFR: JDK-8042981: Strip type annotations in Types' utility methods [v5]
Joe Darcy
darcy at openjdk.org
Fri Jan 19 18:19:35 UTC 2024
On Fri, 12 Jan 2024 20:28:00 GMT, Liam Miller-Cushon <cushon at openjdk.org> wrote:
>> Joe Darcy has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Update implementation and tests.
>
> test/langtools/tools/javac/processing/model/util/types/TestAnnotationStripping.java line 140:
>
>> 138: }
>> 139:
>> 140: void checkEmptyAnnotations(AnnotatedConstruct ac) {
>
> I think it would be valuable to expand the assertion to visit contained types, and assert that none of the contained types have annotations:
>
>
> diff --git a/test/langtools/tools/javac/processing/model/util/types/TestAnnotationStripping.java b/test/langtools/tools/javac/processing/model/util/types/TestAnnotationStripping.java
> index c627f41ba0b..8bb3c7cd3a8 100644
> --- a/test/langtools/tools/javac/processing/model/util/types/TestAnnotationStripping.java
> +++ b/test/langtools/tools/javac/processing/model/util/types/TestAnnotationStripping.java
> @@ -82,10 +82,10 @@ public boolean process(Set<? extends TypeElement> annotations,
>
> System.err.print("\tgetArrayType()");
> ArrayType arrayType = typeUtils.getArrayType(returnType);
> - checkEmptyAnnotations(arrayType);
> /*
> * "Annotations on the component type are preserved."
> */
> + checkEmptyAnnotations((AnnotatedConstruct) arrayType);
> checkEqualTypeAndAnnotations(returnType, arrayType.getComponentType());
>
> if (!returnType.getKind().isPrimitive()) {
> @@ -93,10 +93,14 @@ public boolean process(Set<? extends TypeElement> annotations,
> * "Annotations on the bounds are preserved."
> */
> WildcardType wcType;
> - checkEmptyAnnotations(wcType = typeUtils.getWildcardType(returnType, null));
> + System.err.print("\tgetWildcardType(returnType, null)");
> + wcType = typeUtils.getWildcardType(returnType, null);
> + checkEmptyAnnotations((AnnotatedConstruct) wcType);
> checkEqualTypeAndAnnotations(returnType, wcType.getExtendsBound());
>
> - checkEmptyAnnotations(wcType = typeUtils.getWildcardType(null, returnType));
> + System.err.print("\tgetWildcardType(null, returnType)");
> + wcType = typeUtils.getWildcardType(null, returnType);
> + checkEmptyAnnotations((AnnotatedConstruct) wcType);
> checkEqualTypeAndAnnotations(returnType, wcType.getSuperBound());
> }
>
> @@ -158,6 +162,54 @@ void checkEmptyAnnotations(AnnotatedConstruct ac) {
> }
> }
>
> + void ch...
> I think it would be valuable to expand the assertion to visit contained types, and assert that none of the contained types have annotations:
>
> ```diff
> diff --git a/test/langtools/tools/javac/processing/model/util/types/TestAnnotationStripping.java b/test/langtools/tools/javac/processing/model/util/types/TestAnnotationStripping.java
> index c627f41ba0b..8bb3c7cd3a8 100644
> --- a/test/langtools/tools/javac/processing/model/util/types/TestAnnotationStripping.java
> +++ b/test/langtools/tools/javac/processing/model/util/types/TestAnnotationStripping.java
> @@ -82,10 +82,10 @@ public boolean process(Set<? extends TypeElement> annotations,
>
> System.err.print("\tgetArrayType()");
> ArrayType arrayType = typeUtils.getArrayType(returnType);
> - checkEmptyAnnotations(arrayType);
> /*
> * "Annotations on the component type are preserved."
> */
> + checkEmptyAnnotations((AnnotatedConstruct) arrayType);
> checkEqualTypeAndAnnotations(returnType, arrayType.getComponentType());
>
> if (!returnType.getKind().isPrimitive()) {
> @@ -93,10 +93,14 @@ public boolean process(Set<? extends TypeElement> annotations,
> * "Annotations on the bounds are preserved."
> */
> WildcardType wcType;
> - checkEmptyAnnotations(wcType = typeUtils.getWildcardType(returnType, null));
> + System.err.print("\tgetWildcardType(returnType, null)");
> + wcType = typeUtils.getWildcardType(returnType, null);
> + checkEmptyAnnotations((AnnotatedConstruct) wcType);
> checkEqualTypeAndAnnotations(returnType, wcType.getExtendsBound());
>
> - checkEmptyAnnotations(wcType = typeUtils.getWildcardType(null, returnType));
> + System.err.print("\tgetWildcardType(null, returnType)");
> + wcType = typeUtils.getWildcardType(null, returnType);
> + checkEmptyAnnotations((AnnotatedConstruct) wcType);
> checkEqualTypeAndAnnotations(returnType, wcType.getSuperBound());
> }
>
> @@ -158,6 +162,54 @@ void checkEmptyAnnotations(AnnotatedConstruct ac) {
> }
> }
>
> + void checkEmptyAnnotations(TypeMirror ac) {
> + System.err.println("\t" + ac);
> + if (ac == null) {
> + return;
> + }
> + new SimpleTypeVisitor14<Void, Void>() {
> + @Override
> + protected Void defaultAction(TypeMirror t, Void o) {
> + checkEmptyAnnotations((AnnotatedConstruct) t);
> + return null;
> + }
> +
> + @Override
> + public Void visitArray(ArrayType t, Void o) {
> + scan(t.getComponentType());
> + return super.visitArray(t, o);
> + }
> +
> + @Override
> + public Void visitDeclared(DeclaredType t, Void o) {
> + scan(t.getEnclosingType());
> + t.getTypeArguments().stream().forEach(this::scan);
> + return super.visitDeclared(t, o);
> + }
> +
> + @Override
> + public Void visitTypeVariable(TypeVariable t, Void o) {
> + // the bounds correspond to the type variable declaration, not its use
> + // scan(t.getUpperBound());
> + // scan(t.getLowerBound());
> + return super.visitTypeVariable(t, o);
> + }
> +
> + @Override
> + public Void visitWildcard(WildcardType t, Void o) {
> + scan(t.getExtendsBound());
> + scan(t.getSuperBound());
> + return super.visitWildcard(t, o);
> + }
> +
> + private void scan(TypeMirror t) {
> + if (t != null) {
> + visit(t);
> + }
> + }
> + }.visit(ac);
> + }
> +
> void checkEqualTypeAndAnnotations(TypeMirror tm1, TypeMirror tm2) {
> if (!typeUtils.isSameType(tm1, tm2)) {
> failures++;
> ```
I've added a variation of the new code to the PR. The checks I added are in line with the properties discussed by the API changes in Types. Thanks.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/8984#discussion_r1459485081
More information about the compiler-dev
mailing list