+1. On 14 Oct 2014, at 13:16, Attila Szegedi <[email protected]> wrote:
> > On Oct 14, 2014, at 11:23 AM, Marcus Lagergren <[email protected]> > 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 <[email protected]> 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. >> >
