RFR: JDK-8031373 and 8030079 -- lint warnings in java.util.stream and java.lang.invoke

[Remi Forax]

On 01/17/2014 05:42 PM, Paul Sandoz wrote:
> On Jan 17, 2014, at 5:09 PM, Brian Goetz <brian.goetz at Oracle.COM> 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