Extending the existing StringConcatFactoryInvariants test we can easily
get most basic
combinations covered, along with some basic verification.
This uncovered what appears to be a similar issue in the BC_SB SCF
strategy, which can be
dealt with by coercing the possibly non-String constant to String at
generation time:
http://cr.openjdk.java.net/~redestad/8186500/jdk.01/
/Claes
On 08/21/2017 08:35 PM, Aleksey Shipilev wrote:
On 08/21/2017 08:17 PM, Aleksey Shipilev wrote:
On 08/21/2017 08:06 PM, Paul Sandoz wrote:
On 21 Aug 2017, at 07:48, Claes Redestad <claes.redes...@oracle.com> wrote:
a trivial test[1] invoking the StringConcatFactory.makeConcatWithConstants fails
when providing an Integer as a constant, which appears to be due to failure to
coerce boxed types to the corresponding primitive types when looking up various
methods in StringConcatHelper:
Webrev: http://cr.openjdk.java.net/~redestad/8186500/jdk.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8186500
Simply using Wrapper.asPrimitiveType coerces boxed types to their primitive
counterpart, and is a (semantical) no-op for other types, e.g., String.
Looks good. Perhaps a token test would be useful (maybe hard to test all code
paths here without some combinator test).
Please add tests. We have to at least assert that passing "null" works fine
there -- I am not sure
it does right now. Since we are talking non-String constants, passing
java/lang/Object is a good
test too.
I remembered why we haven't covered this before:
* <li><em>\2 (Unicode point 0002):</em> a constant. This input passed
* through static bootstrap argument. This constant can be any value
* representable in constant pool. If necessary, the factory would call
* {@code toString} to perform a one-time String conversion.</li>
Neither Integer, nor Object are representable in constant pool, and so this bug
cannot surface via
JDK 9 bytecode, but only via direct SCF call. So, from some point of view, SCF
may exhibit undefined
behavior, including throwing up on non-CP-representable value.
Using "should be the value representable in constant pool" instead of "can..."
would make it
stronger, but alas. Once other constant flavors are added (/me looks at Paul,
Brian and John), it
would become a possibility. This is why I think we need to add Integer and
Object constant tests for
future proofing SCF, while this is still a gray area.
...and maybe add {Class, MethodHandle, MethodType} as the tested arguments too,
as per:
https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.4
Thanks for taking care of this!
-Aleksey
