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
