RFR: 8260566: Pattern type X is a subtype of expression type Y message is incorrect

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

RFR: 8260566: Pattern type X is a subtype of expression type Y message is incorrect

Guoxiong Li-2
Hi all,

The order of the arguments is wrong when using method `Errors.InstanceofPatternNoSubtype`.

This patch fixes it and revises the corresponding tests.
Thank you for taking the time to review.
And it may be good to fix JDK16 as well.

Best Regards.
-- xiong

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

Commit messages:
 - 8260566:Pattern type X is a subtype of expression type Y message is incorrect

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

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

Re: RFR: 8260566: Pattern type X is a subtype of expression type Y message is incorrect

Tagir F.Valeev
On Fri, 29 Jan 2021 11:20:06 GMT, Guoxiong Li <[hidden email]> wrote:

> Hi all,
>
> The order of the arguments is wrong when using method `Errors.InstanceofPatternNoSubtype`.
>
> This patch fixes it and revises the corresponding tests.
> Thank you for taking the time to review.
> And it may be good to fix JDK16 as well.
>
> Best Regards.
> -- xiong

The fix doesn't seem correct to me. Consider the code in the ticket:

public class Test {
  void test(String s) {
    if(s instanceof Object obj) {}
  }
}

The previous message was:
Test.java:3: error: pattern type Object is a subtype of expression type String
    if(s instanceof Object obj) {}
It was incorrect because Object is not a subtype of String

After applying this fix, the message will be
Test.java:3: error: pattern type String is a subtype of expression type Object
    if(s instanceof Object obj) {}
It's also incorrect because String is not a pattern type, and Object is not an expression type. A correct message would be
Test.java:3: error: expression type String is a subtype of pattern type Object
or
Test.java:3: error: pattern type Object is a supertype of expression type String

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

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

Re: RFR: 8260566: Pattern type X is a subtype of expression type Y message is incorrect

Guoxiong Li-2
On Fri, 29 Jan 2021 11:59:30 GMT, Tagir F. Valeev <[hidden email]> wrote:

>> Hi all,
>>
>> The order of the arguments is wrong when using method `Errors.InstanceofPatternNoSubtype`.
>>
>> This patch fixes it and revises the corresponding tests.
>> Thank you for taking the time to review.
>> And it may be good to fix JDK16 as well.
>>
>> Best Regards.
>> -- xiong
>
> The fix doesn't seem correct to me. Consider the code in the ticket:
>
> public class Test {
>   void test(String s) {
>     if(s instanceof Object obj) {}
>   }
> }
>
> The previous message was:
> Test.java:3: error: pattern type Object is a subtype of expression type String
>     if(s instanceof Object obj) {}
> It was incorrect because Object is not a subtype of String
>
> After applying this fix, the message will be
> Test.java:3: error: pattern type String is a subtype of expression type Object
>     if(s instanceof Object obj) {}
> It's also incorrect because String is not a pattern type, and Object is not an expression type. A correct message would be
> Test.java:3: error: expression type String is a subtype of pattern type Object
> or
> Test.java:3: error: pattern type Object is a supertype of expression type String

@amaembo You are right. I will fix it later.

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

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

Re: RFR: 8260566: Pattern type X is a subtype of expression type Y message is incorrect [v2]

Guoxiong Li-2
In reply to this post by Guoxiong Li-2
> Hi all,
>
> The order of the arguments is wrong when using method `Errors.InstanceofPatternNoSubtype`.
>
> This patch fixes it and revises the corresponding tests.
> Thank you for taking the time to review.
> And it may be good to fix JDK16 as well.
>
> Best Regards.
> -- xiong

Guoxiong Li has updated the pull request incrementally with one additional commit since the last revision:

  Revise error message.

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2311/files
  - new: https://git.openjdk.java.net/jdk/pull/2311/files/26f93d3a..6bcda73c

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

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2311.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2311/head:pull/2311

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

Re: RFR: 8260566: Pattern type X is a subtype of expression type Y message is incorrect [v2]

Jan Lahoda-3
On Fri, 29 Jan 2021 12:49:06 GMT, Guoxiong Li <[hidden email]> wrote:

>> Hi all,
>>
>> The order of the arguments is wrong when using method `Errors.InstanceofPatternNoSubtype`.
>>
>> This patch fixes it and revises the corresponding tests.
>> Thank you for taking the time to review.
>> And it may be good to fix JDK16 as well.
>>
>> Best Regards.
>> -- xiong
>
> Guoxiong Li has updated the pull request incrementally with one additional commit since the last revision:
>
>   Revise error message.

Looks good to me.

Thanks for Tagir for pointing out the mistake in the error message.

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

Marked as reviewed by jlahoda (Reviewer).

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

Re: RFR: 8260566: Pattern type X is a subtype of expression type Y message is incorrect [v2]

Tagir F.Valeev
In reply to this post by Guoxiong Li-2
On Fri, 29 Jan 2021 12:49:06 GMT, Guoxiong Li <[hidden email]> wrote:

>> Hi all,
>>
>> The order of the arguments is wrong when using method `Errors.InstanceofPatternNoSubtype`.
>>
>> This patch fixes it and revises the corresponding tests.
>> Thank you for taking the time to review.
>> And it may be good to fix JDK16 as well.
>>
>> Best Regards.
>> -- xiong
>
> Guoxiong Li has updated the pull request incrementally with one additional commit since the last revision:
>
>   Revise error message.

Looks good to me! (I'm not an official reviewer though)

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

Marked as reviewed by tvaleev (Committer).

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

Re: RFR: 8260566: Pattern type X is a subtype of expression type Y message is incorrect [v2]

Guoxiong Li-2
On Fri, 29 Jan 2021 13:19:37 GMT, Tagir F. Valeev <[hidden email]> wrote:

>> Guoxiong Li has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Revise error message.
>
> Looks good to me! (I'm not an official reviewer though)

@amaembo @lahodaj Thanks for your review. Could I get your help to sponsor this patch?

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

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

Integrated: 8260566: Pattern type X is a subtype of expression type Y message is incorrect

Guoxiong Li-2
In reply to this post by Guoxiong Li-2
On Fri, 29 Jan 2021 11:20:06 GMT, Guoxiong Li <[hidden email]> wrote:

> Hi all,
>
> The order of the arguments is wrong when using method `Errors.InstanceofPatternNoSubtype`.
>
> This patch fixes it and revises the corresponding tests.
> Thank you for taking the time to review.
> And it may be good to fix JDK16 as well.
>
> Best Regards.
> -- xiong

This pull request has now been integrated.

Changeset: 739bbd03
Author:    Guoxiong Li <[hidden email]>
Committer: Vicente Romero <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/739bbd03
Stats:     5 lines in 4 files changed: 0 ins; 0 del; 5 mod

8260566: Pattern type X is a subtype of expression type Y message is incorrect

Reviewed-by: jlahoda, tvaleev

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

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