Reviewers: rossberg,

https://codereview.chromium.org/15943002/diff/1/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/15943002/diff/1/include/v8.h#newcode2451
include/v8.h:2451: uint8_t GetUint8(size_t byte_offset) const;
I'd be okay with removing these. I added them for the sake of
convenience.

If you keep them, it might be worth it to inline them in v8.h - though
that might make unwrapping the JSDataView kind of icky.

https://codereview.chromium.org/15943002/diff/1/src/dataview.js
File src/dataview.js (right):

https://codereview.chromium.org/15943002/diff/1/src/dataview.js#newcode87
src/dataview.js:87: return %DataViewGetInt8(this, offset, false);
I've been trying to figure out if it's possible to generate inline code
for this but so far I've drawn up blank.  If you can point me in the
right direction, I'll happily follow up with a patch.

https://codereview.chromium.org/15943002/diff/1/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/15943002/diff/1/src/runtime.cc#newcode802
src/runtime.cc:802: #define DATA_VIEW_GETTER(getter, accessor)
                         \
These could be merged / DRY'd with the code in src/api.cc.  I haven't
done that yet because, like I said in another comment, I'd be okay with
dropping the API functions.

https://codereview.chromium.org/15943002/diff/1/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

https://codereview.chromium.org/15943002/diff/1/test/cctest/test-api.cc#newcode2668
test/cctest/test-api.cc:2668: CHECK_EQ(123.456,
data_view->GetFloat64(4));
But if you decide to keep the API functions, there should probably be
some discussion on how to handle NaNs and other edge cases.  The
DataView spec leaves that remarkably wide open, the only requirement is
that it's valid IEEE 754:

" When the not-a-number (NaN) value is stored into a Float32Array or
Float64Array, or into a DataView using the setFloat32 or setFloat64
methods, the bit pattern written into the underlying ArrayBuffer is not
specified, but shall be one of the IEEE 754 bit patterns that represent
NaN [IEEE-754].

When a bit pattern representing an IEEE 754 NaN is loaded from a
Float32Array or Float64Array, or from a DataView using the getFloat32 or
getFloat64 methods, the language binding (for example, ECMAScript) may
use an alternative bit pattern to represent the NaN value."

Description:
v8 typed arrays: add DataView type

Please review this at https://codereview.chromium.org/15943002/

Affected files:
  M include/v8.h
  M src/api.h
  M src/api.cc
  M src/bootstrapper.cc
  M src/contexts.h
  A src/dataview.js
  M src/factory.h
  M src/factory.cc
  M src/flag-definitions.h
  M src/messages.js
  M src/objects-debug.cc
  M src/objects-inl.h
  M src/objects-printer.cc
  M src/objects-visiting.cc
  M src/objects.h
  M src/objects.cc
  M src/runtime.h
  M src/runtime.cc
  M src/utils.h
  M test/cctest/test-api.cc
  A test/mjsunit/harmony/dataview.js
  M tools/gyp/v8.gyp


--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to