Review request for JDK-8060242

[Attila Szegedi]

On Oct 14, 2014, at 11:23 AM, Marcus Lagergren <marcus.lagergren at oracle.com> wrote:

> * final missing in NativeFloat32Array and friends.

Added.

> * I want a unit test that checks that long is the correct representation for uint32 array elements. uint16 should be fine as it's chars I guess. 

The getElementType() was previously only used by NativeArray.PopLinkLogic, so its value in Uint32ArrayData had no effect on anything. Uint32ArrayData getElem() and setElem() have both alway returned/taken long values, so long is the correct value. Which is to say, there's no test that could detect a difference from the behavior before this patch.

> * There is whitespace missing between functions in NativeUint8Array.

Added.

> *  Can you check that all unsigned typed arrays work for boundary conditions.

Uint16 and Uint8 trivially fit into a 32-bit int, so they have to. Existing tests, specifically NASHORN-377 exercise all of these quite thoroughly, also lots of Octane benchmarks too (pdfjs and mandreel use a lot of Uint8, Uint16, Uint32, Float32).

> 
> Otherwise, looking very good. Nicely spotted.
> 
> /M
> 
> On 13 Oct 2014, at 20:34, Attila Szegedi <attila.szegedi at oracle.com> wrote:
> 
>> Please review JDK-8060242 at <http://cr.openjdk.java.net/~attila/8060242/webrev.00> for <https://bugs.openjdk.java.net/browse/JDK-8060242>
>> 
>> The fix is mostly in the TypeEvaluator.java, the rest is just plumbing (ArrayBufferView has to be public so I can us it with instanceof; array data subclasses for buffer views need to support optimistic operations; Int/Long/Number/ObjectArrayData now inherit they getOptimisticType() from ContinuousArrayData; finally there was a genuine data type bug where NativeUint32Array reported its array element type to be int instead of long).
>> 
>> Thanks,
>> Attila.
>