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

Aleksey Shipilev shade at redhat.com
Wed Aug 23 16:35:12 UTC 2017


On 08/23/2017 06:31 PM, Claes Redestad wrote:
> 
> 
> On 08/23/2017 06:31 PM, Aleksey Shipilev wrote:
>> On 08/23/2017 06:26 PM, Claes Redestad wrote:
>>> On 08/23/2017 06:08 PM, Aleksey Shipilev wrote:
>>>>    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?
>> Yes, and I'd probably cascade this through the code: make RecipeElement.value String-typed, make
>> RecipeElement.getValue() return String, remove all excess String conversions, voila? This also makes
>> the code simpler to understand.
> 
> Sure, mind if I defer that to a future RFE, though? :-)

Oh, c'mon, that should be a simple change :) And it makes the patch (that we would have to backport
some day) more readable!

-Aleksey



More information about the core-libs-dev mailing list