Very nice! Only two main comments:
1. As suggested by Toon and disussed offline, let's just use LanguageMode
for
LoadICs.
2. Even more test coverage, see the comments below; also, perhaps a simple
cctest that checks interceptors.
https://codereview.chromium.org/1168093002/diff/160001/src/ic/x64/ic-x64.cc
File src/ic/x64/ic-x64.cc (right):
https://codereview.chromium.org/1168093002/diff/160001/src/ic/x64/ic-x64.cc#newcode194
src/ic/x64/ic-x64.cc:194: Label done, in_bounds, return_absent;
Nit: just "absent", it doesn't necessarily return
https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/load-element-mutate-backing-store.js
File test/mjsunit/strong/load-element-mutate-backing-store.js (right):
https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/load-element-mutate-backing-store.js#newcode7
test/mjsunit/strong/load-element-mutate-backing-store.js:7: //
TODO(conradw): Track implementation of strong bit for other objects, add
See above
https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/load-element-mutate-backing-store.js#newcode10
test/mjsunit/strong/load-element-mutate-backing-store.js:10: function
getObjects() {
See above
https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/load-element-mutate-backing-store.js#newcode202
test/mjsunit/strong/load-element-mutate-backing-store.js:202: for (let
func of readStrong) {
Yeah, a quintuple loop like this is probably producing quite a
long-running test. Maybe you don't need to test the full matrix, but
just vary a few of the dimensions at a time?
https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/load-element.js
File test/mjsunit/strong/load-element.js (right):
https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/load-element.js#newcode7
test/mjsunit/strong/load-element.js:7: // TODO(conradw): Track
implementation of strong bit for other objects, add
Currently, the semantics does not rely on that, does it?
https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/load-element.js#newcode10
test/mjsunit/strong/load-element.js:10: function getObjects() {
Other interesting cases to add:
- arguments (both strict and sloppy)
- typed arrays
- a non-standard builtin instance (e.g. of Date)
- some builtins (Object, Array, Function)
- primitives (numbers, bools, which should be wrapped implicitly)
- proxies (in a separate test file)
https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/load-element.js#newcode200
test/mjsunit/strong/load-element.js:200: // Attempting to access a
property on an objects with no defined properties
Nit: typo
https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/load-property-mutate-backing-store.js
File test/mjsunit/strong/load-property-mutate-backing-store.js (right):
https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/load-property-mutate-backing-store.js#newcode7
test/mjsunit/strong/load-property-mutate-backing-store.js:7: //
TODO(conradw): Track implementation of strong bit for other objects, add
See above
https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/load-property-mutate-backing-store.js#newcode10
test/mjsunit/strong/load-property-mutate-backing-store.js:10: function
getObjects() {
See above
https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/load-property.js
File test/mjsunit/strong/load-property.js (right):
https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/load-property.js#newcode7
test/mjsunit/strong/load-property.js:7: // TODO(conradw): Track
implementation of strong bit for other objects, add
See above
https://codereview.chromium.org/1168093002/diff/200001/test/mjsunit/strong/load-property.js#newcode10
test/mjsunit/strong/load-property.js:10: function getObjects() {
See above
https://codereview.chromium.org/1168093002/
--
--
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/d/optout.
