RFR: 8186500: StringConcatFactory.makeConcatWithConstants throws AssertionError when recipe contains non-String constants

[Claes Redestad]

On 08/23/2017 06:08 PM, Aleksey Shipilev wrote:
> On 08/23/2017 01:52 PM, Claes Redestad wrote:
>> What I wasn't sure about is *when* the String.valueOf should happen, but as
>> makeConcatWithConstants specify "If necessary, the factory would call toString to
>> perform a one-time String conversion" then I think we could (should?) do this at
>> our earliest convenience, which might even be in the RecipeElement construction:
> Yes, I don't see why not.
>
>> http://cr.openjdk.java.net/~redestad/8186500/jdk.02/
> Hm!
>
>   *) Seems that due to cnst being an (autoboxed) reference, isPrimitive is always false on this path?

Ouch...

>   332             Object value = Objects.requireNonNull(cnst);
>   333             if (!value.getClass().isPrimitive()) {
>   334                 this.value = String.valueOf(cnst);
>   335             } else {
>   336                 this.value = value;
>   337             }
>
> ...so that value is always String after this?

Right. Which works, and I'm not sure we really lose much this way. 
Nothing on the
code generated by javac, only theoretical performance points on other 
code. Keep
it simple for now and just do String.valueOf in RecipeElement always?

> ...and that avoids String conversion on RecipeElement.getValue() paths completely?

We could still do String.valueOf on the other end to make sure (as this
is a no-op on Strings):

http://cr.openjdk.java.net/~redestad/8186500/jdk.03

Thanks!

/Claes