Review request: JDK-8157246 MHs.arrayLength, arrayElementGetter/Setter, arrayConstructor need to specify invocation-time behavior

[Roger Riggs]

Looks fine.

Nothing more from me,   Roger

On 11/8/2017 6:52 PM, mandy chung wrote:
> Hi Roger,
>
> Updated version:
> http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8157246/webrev.02/index.html 
>
>
> On 11/8/17 11:37 AM, Roger Riggs wrote:
>> Hi Mandy,
>>
>> A few editorial suggestions:
>>
>> MethodHandle.java:
>> 891: "accepts**a** zero-length trailing array argument
>
> Fixed
>> 895: "if **the* *{@code array} does"
>>
>
> {@code array} is referred to the parameter name where I don't use 
> article.  I drop the {@code...} and change it to "If the array".
>
>> MethodHandleImpl.java:
>> 667:  hard to follow indentation; perhaps adding the "else" will help.
>>
>
> I revised it to the following:
>
>         if (av == null && n == 0) {
>             return;
>         } else if (av == null) {
>             throw new NullPointerException("null array reference");
>
>> MethodHandles.java:
>> 2517: "Produces a method handle *for* constructing arrays..."
>
> It reads fine without "for" and the other arrayXXX factory methods are 
> similar.  I leave it for the original author to follow up.
>
>> 2523: "array size, *a* {@code NegativeArraySizeException} will be 
>> thrown."
>> 2550: "array reference, *a* {@code NullPointerPointer} exception will 
>> be thrown."
>> 2573 & 2598 & 2641: "*A* {@code NullPointerPointer} *exception* will 
>> be thrown if the array reference
>>
>> Is there a test for asSpreader(null, 0) and asSpreader(null, 1)? (The 
>> new exception thrown at MethodHandleImpl:667)
>>
>
> The new exception is thrown when the MethodHandle returned by 
> MethodHandle::asSpreader.  The test cases are covered by the new 
> InvokeMethodHandleWithBadArgument test.
>
> asSpreader(null, 0) and asSpreader(null, 1) are not related to this 
> issue.  The check is done by MethodHandle::asSpreaderChecks. Since 
> this gets my attention, I add the test cases in an existing test 
> SpreadCollectTest.java.
>
> Mandy