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

[Jan Lahoda]

Looks good to me too. Robert, I can push this, unless you want to.

Jan

On 11.12.2015 11:54, Maurizio Cimadamore wrote:
> Looks good to me; also good point about lower bounds... lower bounds are
> no good for LHS in assigments (and I shouldn't make type-system comments
> too late at night) :-)
>
> Maurizio
>
> On 11/12/15 10:06, ShinyaYoshida wrote:
>> Oh... Certainly.
>> I've updated:
>> http://cr.openjdk.java.net/~shinyafox/kulla/8144903/webrev.05/
>> <http://cr.openjdk.java.net/%7Eshinyafox/kulla/8144903/webrev.05/>
>>
>> Thank you,
>> shinyafox(Shinya Yoshida)
>>
>> 2015-12-11 16:52 GMT+09:00 Robert Field <robert.field at oracle.com
>> <mailto:robert.field at oracle.com>>:
>>
>>        91     public String visitClassType(ClassType ct, Locale locale) {
>>        92         try {
>>        93             useWildCard = true;
>>        94             return super.visitClassType(ct, locale);
>>        95         } finally {
>>        96             useWildCard = false;
>>        97         }
>>        98     }
>>
>>     I believe this could give wrong results because with nested class types you will be coming back to a case where useWildCard should still be true.
>>     The thing to do here is save the incoming value of the state you want to change (useWildCard) in a local variable, and restore it in the finally.
>>
>>     -Robert
>>
>>     On 12/10/15 14:13, ShinyaYoshida wrote:
>>>     Hi Maurizio, Thank you.
>>>
>>>     2015-12-11 6:58 GMT+09:00 Maurizio Cimadamore
>>>     <<mailto:maurizio.cimadamore at oracle.com>maurizio.cimadamore at oracle.com>:
>>>
>>>         Looks good, some comments:
>>>
>>>         * watch out for the 'useWildcards' variable - you set it to
>>>         true on the first class you see, and you never reset it to
>>>         false - that could lead to spurious behavior - at least in
>>>         principle (probably not in practice given that if a wildcard
>>>         appears as a toplevel type you never hit visitClassType).
>>>
>>>     I've updated:
>>>     http://cr.openjdk.java.net/~shinyafox/kulla/8144903/webrev.04/
>>>     <http://cr.openjdk.java.net/%7Eshinyafox/kulla/8144903/webrev.04/>
>>>
>>>         * I think you also use the wildcard lower bound, in case of a
>>>         toplevel ? super T. In that case, T is always better than Object.
>>>
>>>     I don't think so, because this printer is to determine a
>>>      suitable variable type which is assigned the expression of the
>>>     "type " which is the argument of this printer.
>>>     And a expression of "? super T" is not assignable to T:
>>>     F<? super String> f = ...
>>>     String s = f.get() // failed to compile
>>>
>>>     Regards,
>>>     shinyafox(Shinya Yoshida)
>>>
>>>
>>>
>>>         Maurizio
>>>
>>>
>>>         On 10/12/15 14:15, ShinyaYoshida wrote:
>>>>         Hi Maurizio and Robert,
>>>>         Thank you for a lot of advice and comments!
>>>>
>>>>         Now I agreed.
>>>>         It is difficult( or impossible) to determine the completely
>>>>         "right" type from bound.
>>>>         And when I work hard around that, the work wouldn't bring
>>>>         benefit what is worth the working to us.
>>>>
>>>>         I'd like propose the fix based on webrev.02:
>>>>         http://cr.openjdk.java.net/~shinyafox/kulla/8144903/webrev.03/
>>>>         <http://cr.openjdk.java.net/%7Eshinyafox/kulla/8144903/webrev.03/>
>>>>
>>>>         In this patch:
>>>>         1. Fix 8144903(this issue)
>>>>         2. Use always wildcard instead of upper bound
>>>>         3. In visitWildcardType, if it is top-level type and it
>>>>         doesn't have extends bound, return "Object"
>>>>         4. In TypePrinter, "Object" appears sometimes so I declare
>>>>         'private static final String OBJECT = "Object"' and use it
>>>>         instead of the literal.
>>>>         5. Add & modify some tests
>>>>
>>>>         Could you review this?
>>>>
>>>>         Best Regards,
>>>>         shinyafox(Shinya Yoshida)
>>>>
>>>>         2015-12-10 19:32 GMT+09:00 Maurizio Cimadamore
>>>>         <<mailto:maurizio.cimadamore at oracle.com>maurizio.cimadamore at oracle.com>:
>>>>
>>>>
>>>>
>>>>             On 10/12/15 03:07, Robert Field wrote:
>>>>>             public static Object $2;
>>>>>
>>>>>             otherwise the generated code will break.
>>>>             What do you mean by this? How is using Object breaking
>>>>             the generated code? Seems to me you have just widened
>>>>             the type of a variable storing an expression - that
>>>>             ought to work; the only problem here is user
>>>>             expectations: if the user is expecting to pass this to a
>>>>             method accepting a CharSequence he/she will be
>>>>             disappointed. But how big of a problem is this? And, as
>>>>             I pointed out elsewhere, no general solution exists for
>>>>             this.
>>>>
>>>>             Maurizio
>>>>
>>>>
>>>
>>>
>>
>>
>