groovy-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Paul King <pa...@asert.com.au>
Subject Re: ShortTypeHandling castToString
Date Sun, 19 Jun 2016 04:29:31 GMT
I updated PR #345 as per your comments (I believe). Let me know if it
looks okay. I did require changing one swing test that was relying on
the old toString behavior. It would be nice if Andres could comment if
such a change could impact Griffon.

On Sat, Jun 18, 2016 at 6:04 PM, Jochen Theodorou <blackdrag@gmx.org> wrote:
> On 15.06.2016 14:36, Winnebeck, Jason wrote:
>>
>> OK, I see the PR. I would be interested to hear Jochen's comments because
>> the way the class is documented it looks like that class is meant to be
>> called in situations with constrained conditions.
>
>
> yes
>
>> I mean, doesn't the normal String cast in Groovy go through
>> DefaultGroovyMethods.asType?
>
>
> no. asType is only for calling asType directly, or using the casting "as". A
> normal cast is done by the means of ScriptBytecodeAdapter#castToType and
> friends. The idea with ShortTypeHandling was to have a class with no dynamic
> method calls and small methods for the easy cases, with reuse in the static
> compiler maybe. The default for a cast using "as", is the normal cast.
>
>> I see in that method there is a specialization when casting to String that
>> calls InvokerHelper.toString, which has a lot of logic in it. For your issue
>> GROOVY-7853 around primitive arrays I see that asType goes to
>> InvokerHelper.toString, which calls InvokerHelper.format, which sees its
>> class "isArray()" is true, then it has a specialization for char[] but if
>> it's not char[] it casts the array to a collection and invokes
>> Invoker.format on that -- which seems to go through a lot of effort casting
>> everything in there recursively to a String when everything is guaranteed to
>> be a primitive at that point.
>>
>> In that sense, even the fix provided in PR 345 doesn't match the standard
>> Groovy cast to String operator, which recursively formats the elements in
>> the array, instead you just use toString.
>>
>> But I also wonder if the real bug is that the compiler generates
>> ShortTypeHandling calls when it should be reverting to the full asType
>> instead.
>
>
> there is also the problem with a dynamically added toString() method.
> Actually GROOVY-7853 makes me think that this method in ShortTypeHandling
> should not be called at all and that the castToType route should be taken
> instead.... and ensured we will pick up the dynamic toString method there as
> well.
>
> bye Jochen
>
>

Mime
View raw message