RFR 8199762: JShell API: Failed to detect override when snippet to be overridden has been changed before

[ShinyaYoshida]

Robert,
Thank you for the review.
Just pushed: http://hg.openjdk.java.net/jdk/jdk/rev/f14852315495

Regards,
shinyafox(Shinya Yoshida)

2018-03-22 8:03 GMT+09:00 Robert Field <robert.field at oracle.com>:

> Looks good!
>
> -Robert
>
>
> On 03/19/18 23:44, ShinyaYoshida wrote:
>
> Hi Robert,
> Thank you for the kind review.
>
> As you mentioned, removing "isNew &&" seems simplest, can keep the
> contract of Key classes and works well.
>
> I've updated the webrev: http://cr.openjdk.java.net/~shinyafox/kulla/
> 8199762/webrev.01/
> Please check this again.
>
> Regards,
> shinyafox(Shinya Yoshida)
>
> 2018-03-19 9:11 GMT+09:00 Robert Field <robert.field at oracle.com>:
>
>> Thanks for reporting this and taking it on.
>>
>> An unwritten design choice is that Key and its subtypes are immutable.
>> note that all Key fields are final.  Mutable fields are held on Snippet and
>> its subtypes.
>>
>> Currently, only "isNew" methods are having their qualified parameter
>> types computed (because modified methods (same signature) can't overwrite
>> methods that haven't already been overwritten by the original).  A
>> side-effect of that though is that though is in cases like this, it is
>> sitting null.  You address this by moving it to MethodKey, but that has the
>> problems mentioned above.
>>
>> For defined methods we always want it set. There seem to be a number of
>> ways this could be done.  I believe, removing the "isNew &&" test would be
>> the simplest.
>>
>> Thanks,
>> Robert
>>
>>
>>
>>
>> On 03/18/18 07:06, ShinyaYoshida wrote:
>>
>>> Hi,
>>>
>>> Please review following:
>>> Webrev: http://cr.openjdk.java.net/~shinyafox/kulla/8199762/webrev/
>>>
>>> Bugs: https://bugs.openjdk.java.net/browse/JDK-8199762
>>>
>>> Regards,
>>> shinyafox(Shinya Yoshida)
>>>
>>
>>
>
>