RFR 8144903: JShell: determine incorrectly the type of the expression which is array type of captured type

[Maurizio Cimadamore]

Hi Shinya,
In my experience, the best way to get to an usable declared type from a 
capture type is to always follow the wildcard - i.e. the upper bound of 
a captured type is more of an internal type system info (the result of 
capture conversion) and I don't think it should be used.

That said, I think there's a problem here: captured vars can appear in 
two places, type position or type-argument position:

1) Type position

class Foo<X> {
    X get() { .. }
}

Foo<? extends String> fs = ...
fs.get(); // you get a #CAP in a type position - i.e. the return type of 
get()

2) Type-argument position

class Foo2<X> {
    List<X> get() { .. }
}

Foo<? extends String> fs = ...
fs.get(); // you get a List<#CAP>, so #CAP is in a type-argument position

I think the TypePrinter should have some logic to detect as to whethe 
you are in case #1 or #2, and if in #1 it should drop wildcards to the 
floor; if it's in #2 it is ok to retain wildcards. So, I think a good 
solution would emit String for #1 and List<? extends String> for #2.

As usual - this is my 0.02$ :-)

[This is essentially an inference problem - we are trying to infer a 
denotable type from the type of an expression which might contain 
non-denotable types - the problem is not too different from what we have 
to do when the functional interfaces that are the target of a lambda 
conversion contains wildcards].

Maurizio

On 09/12/15 00:34, Robert Field wrote:
> Exactly, Shinya. Still curious of Jan's thinking.
>
> -Robert
>
>
>
> On December 8, 2015 15:13:42 ShinyaYoshida <bitterfoxc at gmail.com> wrote:
>
>> Hi Robert,
>> Thank you for your review and comments.
>>
>> 2015-12-09 5:28 GMT+09:00 Robert Field <robert.field at oracle.com>:
>>
>>> [Jan, I'd like your opinion]
>>>
>>> Thank you for seeing this and drilling into where the problem is.  
>>> This is
>>> important to fix.
>>>
>>> I see that this would address the problem.  However, I am squeamish 
>>> about
>>> the solution as it unwraps to match the result of layers of underlying
>>> super class behavior.  Looking at it further, I believe the problem 
>>> is in
>>> the pre-existing typeToPrint, my bad!  I don't think the approach I 
>>> took is
>>> right.  Stepping back, we want upper bound unless the type is 
>>> embedded in a
>>> type that takes generic types, class types.  That suggests to me 
>>> replacing
>>> typeToPrint with a boolean useWildCard = false, which would be 
>>> changed by
>>> visitClassType().  But alas this is recursive and this visitor does not
>>> have state that passes through the recursion, so it would be global 
>>> -- in
>>> this case that should work, but ugly.
>>>
>> Certainly, my fix is too depend on the super implementation.
>> The fix which you mentioned is like this, right?
>> http://cr.openjdk.java.net/~shinyafox/kulla/8144903/webrev.01_0/
>>
>>
>>
>>> Ah! Really the problem is that we want the top-level type to be erased.
>>> But that means holding an instance of Types.  Deeper integration with
>>> javac.  Hmmmm....
>>>
>> Another possible fix is making TypePrinter general(no recursive 
>> state) and
>> adding the print method what is erase top-level capture to TypePrinter,
>> using the print method at callsite:
>> http://cr.openjdk.java.net/~shinyafox/kulla/8144903/webrev.01_1/
>>
>> Regards,
>> shinyafox(Shinya Yoshida)
>>
>>
>>>
>>>
>>> -Robert
>>>
>>> On Dec 8, 2015, at 1:10 AM, ShinyaYoshida <bitterfoxc at gmail.com> wrote:
>>>
>>> Hi Robert,
>>> Could you review my patch for
>>> "JShell: determine incorrectly the type of the expression which is 
>>> array
>>> type of captured type"?
>>> https://bugs.openjdk.java.net/browse/JDK-8144903
>>>
>>> webrev: http://cr.openjdk.java.net/~shinyafox/kulla/8144903/webrev.00/
>>>
>>> Regards,
>>> shinyafox(Shinya Yoshida)
>>>
>>>
>>>