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

[ShinyaYoshida]

Hi Maurizio and Jan,
Thank you for your advice.
I've tried to unroll it in wildcard to take the advice.
http://cr.openjdk.java.net/~shinyafox/kulla/8144903/webrev.02/

But I've met a problem.
In this case, we get the Object instead of CharSequence, this is not
acceptable for user:

In TypeNameTest#testBounds
        assertEval("class Test1<X extends CharSequence> { public X get() {
return null; } }");
        Snippet x = varKey(assertEval("Test1<?> test1 = new Test1<>();"));
        VarSnippet sn4 = (VarSnippet) varKey(assertEval("test1.get()"));
        assertEquals(sn4.typeName(), "CharSequence");

Compiler determines the upper bound of wildcard"?" as CharSequence from the
declaration.
But WildcardType doesn't have that information at all.

Do you have any idea for this?

Regards,
shinyafox(Shinya Yoshida)


2015-12-10 0:00 GMT+09:00 Jan Lahoda <jan.lahoda at oracle.com>:

> 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)
>>>>>
>>>>>
>>>>>
>>>>>
>>