RFR: JDK-8042981: Strip type annotations in Types' utility methods [v2]

[Liam Miller-Cushon]

On Thu, 6 Apr 2023 04:58:16 GMT, Joe Darcy <darcy at openjdk.org> wrote:

>> Joe Darcy has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
>> 
>>  - Update visitor; all langtools regression tests pass.
>>  - Merge branch 'master' into JDK-8042981
>>  - JDK-8042981: Strip type annotations in Types' utility methods
>
> Still keep-alive.

@jddarcy I had another look at this prompted by your comment in https://github.com/openjdk/jdk/pull/16578#issuecomment-1805032818, and I have a theory about why `stripMetadata()` isn't working reliably.

`stripMetadata()` uses `typeNoMetadata()` to remove the metadata, which is implemented by using `baseType()` to get the original type before metadata was added. That works if all logic that adds metadata uses `cloneWithMetadata`, which creates a subclass that overrides `baseType()` to return the original type without metadata:

https://github.com/openjdk/jdk/blob/c9077b8b816d2efe4559c71341228a8dc319604f/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Type.java#L748-L751

However there are a few places metadata is annotated to newly constructed type directly, and `baseType()` isn't overridden to return an unannotated type.

I was able to enable the disables test cases in this PR and get them passing by debugging for locations where a `Type` is constructed and `baseType()` has non-empty metadata, and then modifying the locations where those types were constructed to set up `baseType()` correctly. I pushed the demo to a branch: https://github.com/cushon/jdk/commit/5910db7e42da8e67e703accfa8adbdb00ce429db

That isn't a proposal for a complete fix, but I think it illustrates what the problem is. There are other places types are constructed that don't set up `baseType()`. This makes me suspicious that the other uses of `baseType()` may not be working as expected.

I think that perhaps a better fix is to use a different approach for removing annotations from types in `Types` utility methods, which doesn't rely on the fragile association between types and an unannotated base type. The tests also pass with the following implementation:


    public Type stripMetadata2() {
        return accept(stripMetadata2, null);
    }
    private static final TypeMapping<Void> stripMetadata2 = new StructuralTypeMapping<>() {
        @Override
        public Type visitClassType(ClassType t, Void aVoid) {
            return super.visitClassType((ClassType)dropAnnotations(t), aVoid);
        }

        @Override
        public Type visitArrayType(ArrayType t, Void aVoid) {
            return super.visitArrayType((ArrayType)dropAnnotations(t), aVoid);
        }

        @Override
        public Type visitWildcardType(WildcardType wt, Void aVoid) {
            return super.visitWildcardType((WildcardType) dropAnnotations(wt), aVoid);
        }

        @Override
        public Type visitType(Type t, Void aVoid) {
            return dropAnnotations(t);
        }

        private static Type dropAnnotations(Type t) {
            return t.getMetadata(Annotations.class) != null ? t.dropMetadata(Annotations.class) : t;
        }

    };

-------------

PR Comment: https://git.openjdk.org/jdk/pull/8984#issuecomment-1806211798