RFR: 8264696: Multi-catch clause causes compiler exception because it uses the package-private supertype

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8264696: Multi-catch clause causes compiler exception because it uses the package-private supertype

Guoxiong Li-2
Hi all,

The method `isAccessible(Env<AttrContext> env, Type t, boolean checkInner)` should take `UnionClassType` as a special type to do some corresponding work. If not, the problem will occur. This patch fixes it and adds a test case.

Thank you for taking the time to review.

Best Regards.
-- Guoxiong

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

Commit messages:
 - 8264696: Multi-catch clause causes compiler exception because it uses the package-private supertype

Changes: https://git.openjdk.java.net/jdk/pull/3374/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3374&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8264696
  Stats: 118 lines in 2 files changed: 115 ins; 0 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3374.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3374/head:pull/3374

PR: https://git.openjdk.java.net/jdk/pull/3374
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264696: Multi-catch clause causes compiler exception because it uses the package-private supertype

Vicente Romero-3
On Wed, 7 Apr 2021 12:26:34 GMT, Guoxiong Li <[hidden email]> wrote:

> Hi all,
>
> The method `isAccessible(Env<AttrContext> env, Type t, boolean checkInner)` should take `UnionClassType` as a special type to do some corresponding work. If not, the problem will occur. This patch fixes it and adds a test case.
>
> Thank you for taking the time to review.
>
> Best Regards.
> -- Guoxiong

test/langtools/tools/javac/resolve/PackagePrivateSupertypeAtMultiCatch.java line 105:

> 103:                 .outdir(out)
> 104:                 .run()
> 105:                 .writeAll();

nit: you don't need the `writeAll()` method given that you are not collecting the output

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

PR: https://git.openjdk.java.net/jdk/pull/3374
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264696: Multi-catch clause causes compiler exception because it uses the package-private supertype

Vicente Romero-3
In reply to this post by Guoxiong Li-2
On Wed, 7 Apr 2021 12:26:34 GMT, Guoxiong Li <[hidden email]> wrote:

> Hi all,
>
> The method `isAccessible(Env<AttrContext> env, Type t, boolean checkInner)` should take `UnionClassType` as a special type to do some corresponding work. If not, the problem will occur. This patch fixes it and adds a test case.
>
> Thank you for taking the time to review.
>
> Best Regards.
> -- Guoxiong

Changes requested by vromero (Reviewer).

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java line 390:

> 388:         if (t.hasTag(ARRAY)) {
> 389:             return isAccessible(env, types.cvarUpperBound(types.elemtype(t)));
> 390:         } else if (!t.isUnion()) {

I would change the code a bit, like in:

        } else if (t.isUnion()) {
            return StreamSupport.stream(((UnionClassType) t).getAlternativeTypes().spliterator(), false)
                    .allMatch(alternative -> isAccessible(env, alternative.tsym, checkInner));
        } else {
            return isAccessible(env, t.tsym, checkInner);
        }

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

PR: https://git.openjdk.java.net/jdk/pull/3374
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264696: Multi-catch clause causes compiler exception because it uses the package-private supertype

Guoxiong Li-2
On Thu, 8 Apr 2021 02:42:26 GMT, Vicente Romero <[hidden email]> wrote:

>> Hi all,
>>
>> The method `isAccessible(Env<AttrContext> env, Type t, boolean checkInner)` should take `UnionClassType` as a special type to do some corresponding work. If not, the problem will occur. This patch fixes it and adds a test case.
>>
>> Thank you for taking the time to review.
>>
>> Best Regards.
>> -- Guoxiong
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java line 390:
>
>> 388:         if (t.hasTag(ARRAY)) {
>> 389:             return isAccessible(env, types.cvarUpperBound(types.elemtype(t)));
>> 390:         } else if (!t.isUnion()) {
>
> I would change the code a bit, like in:
>
>         } else if (t.isUnion()) {
>             return StreamSupport.stream(((UnionClassType) t).getAlternativeTypes().spliterator(), false)
>                     .allMatch(alternative -> isAccessible(env, alternative.tsym, checkInner));
>         } else {
>             return isAccessible(env, t.tsym, checkInner);
>         }

Very nice suggestion. Fixed.

> test/langtools/tools/javac/resolve/PackagePrivateSupertypeAtMultiCatch.java line 105:
>
>> 103:                 .outdir(out)
>> 104:                 .run()
>> 105:                 .writeAll();
>
> nit: you don't need the `writeAll()` method given that you are not collecting the output

Fixed.

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

PR: https://git.openjdk.java.net/jdk/pull/3374
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264696: Multi-catch clause causes compiler exception because it uses the package-private supertype [v2]

Guoxiong Li-2
In reply to this post by Guoxiong Li-2
> Hi all,
>
> The method `isAccessible(Env<AttrContext> env, Type t, boolean checkInner)` should take `UnionClassType` as a special type to do some corresponding work. If not, the problem will occur. This patch fixes it and adds a test case.
>
> Thank you for taking the time to review.
>
> Best Regards.
> -- Guoxiong

Guoxiong Li has updated the pull request incrementally with two additional commits since the last revision:

 - Use stream api
 - Remove writeAll()

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3374/files
  - new: https://git.openjdk.java.net/jdk/pull/3374/files/2e0dba0f..d6a8b12e

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3374&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3374&range=00-01

  Stats: 11 lines in 2 files changed: 2 ins; 5 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3374.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3374/head:pull/3374

PR: https://git.openjdk.java.net/jdk/pull/3374
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264696: Multi-catch clause causes compiler exception because it uses the package-private supertype [v2]

Vicente Romero-3
On Thu, 8 Apr 2021 05:13:38 GMT, Guoxiong Li <[hidden email]> wrote:

>> Hi all,
>>
>> The method `isAccessible(Env<AttrContext> env, Type t, boolean checkInner)` should take `UnionClassType` as a special type to do some corresponding work. If not, the problem will occur. This patch fixes it and adds a test case.
>>
>> Thank you for taking the time to review.
>>
>> Best Regards.
>> -- Guoxiong
>
> Guoxiong Li has updated the pull request incrementally with two additional commits since the last revision:
>
>  - Use stream api
>  - Remove writeAll()

looks good, thanks for the fix

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

Marked as reviewed by vromero (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3374
Reply | Threaded
Open this post in threaded view
|

Integrated: 8264696: Multi-catch clause causes compiler exception because it uses the package-private supertype

Guoxiong Li-2
In reply to this post by Guoxiong Li-2
On Wed, 7 Apr 2021 12:26:34 GMT, Guoxiong Li <[hidden email]> wrote:

> Hi all,
>
> The method `isAccessible(Env<AttrContext> env, Type t, boolean checkInner)` should take `UnionClassType` as a special type to do some corresponding work. If not, the problem will occur. This patch fixes it and adds a test case.
>
> Thank you for taking the time to review.
>
> Best Regards.
> -- Guoxiong

This pull request has now been integrated.

Changeset: 57f1e7d9
Author:    Guoxiong Li <[hidden email]>
Committer: Vicente Romero <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/57f1e7d9
Stats:     115 lines in 2 files changed: 112 ins; 0 del; 3 mod

8264696: Multi-catch clause causes compiler exception because it uses the package-private supertype

Reviewed-by: vromero

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

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