RFR: JDK-8189248: Jshell /open from file generate error when it tries to upcast to typed instance of other with type inferred from it

[Robert Field]

Thumbs up!

-Robert



On December 6, 2017 6:12:20 AM Jan Lahoda <jan.lahoda at oracle.com> wrote:

> On 5.12.2017 21:43, Robert Field wrote:
>> I believe you want the second argument to setWrap to be “legit” rather than 
>> “ins” (as on line 807) so that you don’t attempt to import failed snippet 
>> names.
>
> Ok, done.
>
>>
>> Otherwise looks fine.
>>
>> It would be good to upgrade the issue Subject so that it will make sense in 
>> the future (and include your comments, below).
>
> Done.
>
> Updated webrev:
> http://cr.openjdk.java.net/~jlahoda/8189248/webrev.01/
>
> Jan
>
>>
>> -Robert
>>
>> P.S. Apologies, the logic of compileAndLoad is super twisty, I/we should 
>> think about a design review.
>>
>>
>>> On Dec 5, 2017, at 6:02 AM, Jan Lahoda <jan.lahoda at oracle.com> wrote:
>>>
>>> Hi,
>>>
>>> Consider these snippet sent (in this order) to jshell:
>>> ---
>>> public class A<T> {
>>>   private class SomeClass {}
>>>   public Supplier<T> m() {
>>>     return new B<>(this);
>>>   }
>>> }
>>> ---
>>>
>>> (note this depends on the future snippet declaring class B)
>>>
>>> ---
>>> public class B<T> implements Supplier<T> {
>>>   public B(A<T> a) {}
>>>   public T get() {return null;}
>>> }
>>> ---
>>>
>>> Eval.compileAndLoad will after a few rounds try to compile inputs like:
>>> ---
>>> package REPL;
>>>
>>> import java.util.function.Supplier;
>>> import static REPL.$JShell$3.B;
>>>
>>> class $JShell$2B {
>>>
>>>     public static class A<T>
>>> ...
>>> }
>>> ---
>>> package REPL;
>>>
>>> import java.util.function.Supplier;
>>> import static REPL.$JShell$2.A;
>>>
>>> class $JShell$3 {
>>>
>>>     public static class B<T> implements Supplier<T>
>>> ...
>>> }
>>> ----
>>>
>>> Please note that "B" is using the old (pre-Unit.markForReplacement()) 
>>> version of A from $JShell$2. While the new declaration is inside 
>>> $JShell$2B. I think this is because the classnames are updates only as 
>>> setWrap(s) run, so the wraps that run first may still see old classnames 
>>> for the latter snippets.
>>>
>>> The proposal is to ensure the proper classnames are set before trying to run:
>>> ins.stream().forEach(u -> u.setWrap(ins, ins));
>>> which creates the real wraps that are processed.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8189248
>>> Webrev: http://cr.openjdk.java.net/~jlahoda/8189248/webrev.00/
>>>
>>> How does it look?
>>>
>>> Thanks,
>>>    Jan
>>