On 01/17/2014 05:42 PM, Paul Sandoz wrote:
On Jan 17, 2014, at 5:09 PM, Brian Goetz <[email protected]> wrote:
Webrev at:
http://cr.openjdk.java.net/~briangoetz/JDK-8031373/webrev/
If someone with ASM fu (Remi?) could sanity check the JLI changes, that would
be appreciated.
Stream code looks good.
I notice some other things in StreamSpliterators that could potentially be
cleaned up too:
- Might be a redundant cast:
312 ph.wrapAndCopyInto((Sink<P_OUT>) consumer::accept,
spliterator);
- Error in JavaDoc:
1324 * The {@coe tryAdvance} method always returns true.
--
IIUC the ASM related updates are due to the deprecated method. The new method
allows for invokespecial/static on methods of interfaces and placing the
correct data in the constant pool. In which case i would expect all the updates
to have a false as the last parameter, which is so. Still, as say Remi's eyes
would be useful on this one.
Paul.
The ASM part is fine.
As Paul said, the last parameter should be true if the owner of the
method call is an interface,
so in case of an INVOKEVIRTUAL, it's always false, in case of an
INVOKEINTERFACE, it's always true,
for INVOKESPECIAL, if it's a call to a constructor ("<init>"), it's
always false.
Given that the code either create classes or call wrapper classes (for
boxing/unboxing).
All calls should use false.
<Nitpicking mode="on">
in TypeConvertingMethodAdapter, I think it's better if false was written
on its own line like the other parameters.
visitMethodInsn(Opcodes.INVOKESTATIC,
wrapperName(w),
NAME_BOX_METHOD,
- boxingDescriptor(w));
+ boxingDescriptor(w),
+ false);
</nitpicking>
cheers,
Rémi
