RFR: 8265899: Use pattern matching for instanceof at module jdk.compiler(part 1) [v2]

Guoxiong Li gli at openjdk.java.net
Mon Apr 26 15:31:29 UTC 2021


On Mon, 26 Apr 2021 13:32:43 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Guoxiong Li has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Cleanup method JavacScope.equals
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/api/JavacScope.java line 130:
> 
>> 128: 
>> 129:     public boolean equals(Object other) {
>> 130:         if (other instanceof JavacScope javacScope) {
> 
> Can this be:
> 
> 
> return other instanceof JavacScope javacScope &&
>             env.equals(javacScope.env) &&
>             isStarImportScope() == javacScope.isStarImportScope();

Fixed

> src/jdk.compiler/share/classes/com/sun/tools/javac/file/JavacFileManager.java line 963:
> 
>> 961:         ArrayList<PathFileObject> result;
>> 962:         if (paths != null) {
>> 963:             result = new ArrayList<>(paths.size());
> 
> This seems odd in the original code - I'd rather have somebody versed in the API double check this - @jonathan-gibbons  ? Anyway - this doesn't seem to be related with using pattern matching, so perhaps it's preferrable to just move it out of this PR.

I think that removing the unnecessary `instanceof` expressions is also the goal of this cleanup. So I want to keep it.

In this case, my logical analysis is shown below.

1. Please see the code below. The parameter type of the method `getJavaFileObjectsFromPaths` is `Collection<? extends Path>`, so we don't need to use `paths instanceof Collection<?>`. We only need to judge whether `paths` is `null`. So I use `paths != null` instead.


    public Iterable<? extends JavaFileObject> getJavaFileObjectsFromPaths(
        Collection<? extends Path> paths)


2. Because the `paths` may be `null`, I move the following code from top scope to `if (paths != null) ` branch scope to avoid the NPE.


        for (Path p: paths)
            result.add(PathFileObject.forSimplePath(this,
                    fsInfo.getCanonicalFile(p), p));


In my opinion, these two steps are reasonable for removing the `instanceof` expression.

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

PR: https://git.openjdk.java.net/jdk/pull/3673


More information about the compiler-dev mailing list