Right,
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
construction:
http://cr.openjdk.java.net/~redestad/8186500/jdk.02/
Reworked the test to be easier to add constant types. Null constants are
specified
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).
WDYT?
/Claes
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
