Mostly looking good, except that it is missing plenty of tests. Some
features
are obviously not working yet (e.g. equality, typeof, etc), but for the
value
behaviour that this CL implements we need tests (in
test/mjsunit/harmony/simd.js), e.g.:
- variants of constructor calls
- prototype identities
- object wrapping
- ClassOf
- explicit & implicit conversions (especially to primitive types)
See e.g. the first half of test/mjsunit/es6/symbols.js for guidance.
I assume the actual SIMD methods are already tested in the SIMD suite.
https://codereview.chromium.org/1160443009/diff/140001/include/v8.h
File include/v8.h (right):
https://codereview.chromium.org/1160443009/diff/140001/include/v8.h#newcode3836
include/v8.h:3836: class V8_EXPORT Float32x4 : public Object {
SIMDs aren't objects, so this needs to derive from Value.
https://codereview.chromium.org/1160443009/diff/140001/src/harmony-simd.js
File src/harmony-simd.js (right):
https://codereview.chromium.org/1160443009/diff/140001/src/harmony-simd.js#newcode113
src/harmony-simd.js:113: %FunctionSetPrototype(GlobalFloat32x4, new
GlobalObject());
Nit: s/new GlobalObject()/{}/
https://codereview.chromium.org/1160443009/diff/140001/src/harmony-simd.js#newcode119
src/harmony-simd.js:119: %AddNamedProperty(GlobalFloat32x4,
symbolToStringTag, 'float32x4', READ_ONLY | DONT_ENUM);
It seems inconsistent that the tag is lower-case, it is upper for all
other types (this is the name of the wrapper type).
Also nit: line length
https://codereview.chromium.org/1160443009/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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 v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
