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

[Jan Lahoda]

I am looking at:
http://hg.netbeans.org/jet-main/file/default/java.source.base/src/org/netbeans/api/java/source/SourceUtils.java#l1067

(which is solving a similar problem; and while I found out there may be 
a bug related to arrays in that.)

I concur with Maurizio that CapturedType.wildcard should (presumably) be 
used always for CapturedTypes. When the resulting wildcard is not 
suitable for the given place (i.e. top-level type), then we should use 
getExtendsBound()/getSuperBound() to unroll the wildcard.

Out of the recent two patches, I like webrev.01_0 much more. Presumably 
just override visitWildcardType and unroll the wildcard if needed, and 
change the visitCapturedType to just:
return visit(t.wildcard, locale);

Would that work?

Jan

On 9.12.2015 13:39, Maurizio Cimadamore wrote:
> 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)
>>>>
>>>>
>>>>
>