[v8-dev] Re: [strong] Implement strong mode restrictions on property access (issue 1168093002 by conr...@chromium.org)

2015-06-18 Thread conradw
https://codereview.chromium.org/1168093002/diff/420001/src/lookup.h File src/lookup.h (right): https://codereview.chromium.org/1168093002/diff/420001/src/lookup.h#newcode227 src/lookup.h:227: On 2015/06/18 10:32:12, Toon Verwaest wrote: spurious change Done.

[v8-dev] Re: [strong] Implement strong mode restrictions on property access (issue 1168093002 by conr...@chromium.org)

2015-06-18 Thread commit-...@chromium.org via codereview.chromium.org
Committed patchset #23 (id:460001) https://codereview.chromium.org/1168093002/ -- -- 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

[v8-dev] Re: [strong] Implement strong mode restrictions on property access (issue 1168093002 by conr...@chromium.org)

2015-06-18 Thread commit-...@chromium.org via codereview.chromium.org
Patchset 23 (id:??) landed as https://crrev.com/85dbfb9a389e7b21bd2a63862202ee97fc5d7982 Cr-Commit-Position: refs/heads/master@{#29109} https://codereview.chromium.org/1168093002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this

[v8-dev] Re: [strong] Implement strong mode restrictions on property access (issue 1168093002 by conr...@chromium.org)

2015-06-18 Thread commit-...@chromium.org via codereview.chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1168093002/460001 https://codereview.chromium.org/1168093002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are

[v8-dev] Re: [strong] Implement strong mode restrictions on property access (issue 1168093002 by conr...@chromium.org)

2015-06-18 Thread conradw
A revert of this CL (patchset #23 id:460001) has been created in https://codereview.chromium.org/1189153002/ by conr...@chromium.org. The reason for reverting is: Speculative revert, maybe breaks GC-stress

[v8-dev] Re: [strong] Implement strong mode restrictions on property access (issue 1168093002 by conr...@chromium.org)

2015-06-18 Thread verwaest
lgtm https://codereview.chromium.org/1168093002/diff/420001/src/lookup.h File src/lookup.h (right): https://codereview.chromium.org/1168093002/diff/420001/src/lookup.h#newcode227 src/lookup.h:227: spurious change https://codereview.chromium.org/1168093002/ -- -- v8-dev mailing list

[v8-dev] Re: [strong] Implement strong mode restrictions on property access (issue 1168093002 by conr...@chromium.org)

2015-06-11 Thread rossberg
LGTM modulo comments, but Toon Michael Stanton should sign off as well. https://codereview.chromium.org/1168093002/diff/21/test/mjsunit/strong/load-element.js File test/mjsunit/strong/load-element.js (right):

[v8-dev] Re: [strong] Implement strong mode restrictions on property access (issue 1168093002 by conr...@chromium.org)

2015-06-11 Thread conradw
https://codereview.chromium.org/1168093002/diff/21/test/mjsunit/strong/load-element.js File test/mjsunit/strong/load-element.js (right): https://codereview.chromium.org/1168093002/diff/21/test/mjsunit/strong/load-element.js#newcode10 test/mjsunit/strong/load-element.js:10: function

[v8-dev] Re: [strong] Implement strong mode restrictions on property access (issue 1168093002 by conr...@chromium.org)

2015-06-10 Thread conradw
Currently refactoring everything back to use language mode instead of strength, other changes are here. https://codereview.chromium.org/1168093002/diff/160001/src/ic/x64/ic-x64.cc File src/ic/x64/ic-x64.cc (right):

[v8-dev] Re: [strong] Implement strong mode restrictions on property access (issue 1168093002 by conr...@chromium.org)

2015-06-10 Thread rossberg
On 2015/06/10 08:28:08, rossberg wrote: https://codereview.chromium.org/1168093002/diff/21/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,

[v8-dev] Re: [strong] Implement strong mode restrictions on property access (issue 1168093002 by conr...@chromium.org)

2015-06-10 Thread rossberg
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.

[v8-dev] Re: [strong] Implement strong mode restrictions on property access (issue 1168093002 by conr...@chromium.org)

2015-06-10 Thread conradw
PTAL, all ports now completed https://codereview.chromium.org/1168093002/ -- -- 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

[v8-dev] Re: [strong] Implement strong mode restrictions on property access (issue 1168093002 by conr...@chromium.org)

2015-06-10 Thread mstarzinger
LGTM on the TurboFan part, didn't look at the rest, please wait on approval from other reviewers as well. https://codereview.chromium.org/1168093002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are

[v8-dev] Re: [strong] Implement strong mode restrictions on property access (issue 1168093002 by conr...@chromium.org)

2015-06-09 Thread mstarzinger
https://codereview.chromium.org/1168093002/diff/120001/src/compiler/js-operator.cc File src/compiler/js-operator.cc (right): https://codereview.chromium.org/1168093002/diff/120001/src/compiler/js-operator.cc#newcode194 src/compiler/js-operator.cc:194: lhs.contextual_mode() ==

[v8-dev] Re: [strong] Implement strong mode restrictions on property access (issue 1168093002 by conr...@chromium.org)

2015-06-09 Thread conradw
https://codereview.chromium.org/1168093002/diff/120001/src/code-factory.h File src/code-factory.h (right): https://codereview.chromium.org/1168093002/diff/120001/src/code-factory.h#newcode44 src/code-factory.h:44: LanguageMode language_mode); On 2015/06/09 10:56:11, Michael Starzinger wrote:

[v8-dev] Re: [strong] Implement strong mode restrictions on property access (issue 1168093002 by conr...@chromium.org)

2015-06-09 Thread mstarzinger
https://codereview.chromium.org/1168093002/diff/120001/src/code-factory.h File src/code-factory.h (right): https://codereview.chromium.org/1168093002/diff/120001/src/code-factory.h#newcode44 src/code-factory.h:44: LanguageMode language_mode); nit: I think the language mode should be the second

[v8-dev] Re: [strong] Implement strong mode restrictions on property access (issue 1168093002 by conr...@chromium.org)

2015-06-09 Thread verwaest
https://codereview.chromium.org/1168093002/diff/140001/src/ic/ic-state.h File src/ic/ic-state.h (right): https://codereview.chromium.org/1168093002/diff/140001/src/ic/ic-state.h#newcode205 src/ic/ic-state.h:205: class ContextualModeBits : public BitFieldContextualMode, 0, 1 {}; Why was this

[v8-dev] Re: [strong] Implement strong mode restrictions on property access (issue 1168093002 by conr...@chromium.org)

2015-06-09 Thread mvstanton
Hi Conrad, ic stuff looks pretty good, a couple comments. https://codereview.chromium.org/1168093002/diff/140001/src/builtins.h File src/builtins.h (right): https://codereview.chromium.org/1168093002/diff/140001/src/builtins.h#newcode93 src/builtins.h:93: LoadICState::kStrongModeState)

[v8-dev] Re: [strong] Implement strong mode restrictions on property access (issue 1168093002 by conr...@chromium.org)

2015-06-09 Thread conradw
https://codereview.chromium.org/1168093002/diff/140001/src/builtins.h File src/builtins.h (right): https://codereview.chromium.org/1168093002/diff/140001/src/builtins.h#newcode93 src/builtins.h:93: LoadICState::kStrongModeState) \ On 2015/06/09 12:46:44, mvstanton wrote: You

[v8-dev] Re: [strong] Implement strong mode restrictions on property access (issue 1168093002 by conr...@chromium.org)

2015-06-09 Thread verwaest
https://codereview.chromium.org/1168093002/diff/140001/src/objects.cc File src/objects.cc (right): https://codereview.chromium.org/1168093002/diff/140001/src/objects.cc#newcode130 src/objects.cc:130: MaybeHandleObject Object::GetProperty(LookupIterator* it, Strength strength) { On 2015/06/09