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

Claes Redestad claes.redestad at oracle.com
Wed Aug 23 11:52:40 UTC 2017


the Wrapper.* code appears to work fine, but makeConcatWithConstants has
pre-existing issues with non-primitive, non-String constants.

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 


Reworked the test to be easier to add constant types. Null constants are 
to throw NPE, so the test now checks that (this might be a needlessly 
strict thing
to specify, but changing the method specification is out of scope here, 
I think).



On 08/22/2017 08:49 PM, Aleksey Shipilev wrote:
> On 08/22/2017 07:10 PM, Claes Redestad wrote:
>> On 2017-08-22 18:34, Aleksey Shipilev wrote:
>>>> http://cr.openjdk.java.net/~redestad/8186500/jdk.01/
>>> Still think testing for {null, Class, MethodHandle, MethodType} would cover more interesting corner
>>> cases.
>> Do you mean as constant arguments? And what should happen in each of these cases?
> As Remi says, should be equivalent to String.valueOf().
> null: should be converted to "null"
> {Class, MH, MT}: should be equivalent to Class::toString
> My concern is that we have to check the new code, e.g. Wrapper.* does not barf when we pass
> something non-primitive, non-String there.
> Thanks,
> -Aleksey

More information about the core-libs-dev mailing list