[v8-dev] Re: [strong] Disallow implicit casts in subtraction (issue 1092353002 by conr...@chromium.org)

2015-04-24 Thread conradw
https://codereview.chromium.org/1092353002/diff/340001/src/ic/ic.cc File src/ic/ic.cc (right): https://codereview.chromium.org/1092353002/diff/340001/src/ic/ic.cc#newcode2810 src/ic/ic.cc:2810: if (is_strong(language_mode)) { On 2015/04/24 14:14:23, arv wrote: Why is this needed? The two branch

[v8-dev] Re: [strong] Disallow implicit casts in subtraction (issue 1092353002 by conr...@chromium.org)

2015-04-24 Thread arv
LGTM Since `git cl format` does not check JavaScript I used the arv linter instead. https://codereview.chromium.org/1092353002/diff/340001/src/ic/ic-state.h File src/ic/ic-state.h (right): https://codereview.chromium.org/1092353002/diff/340001/src/ic/ic-state.h#newcode151 src/ic/ic-state.h:

[v8-dev] Re: [strong] Disallow implicit casts in subtraction (issue 1092353002 by conr...@chromium.org)

2015-04-24 Thread commit-bot
Patchset 9 (id:??) landed as https://crrev.com/ae7ce701aef2356424bb35ace91cd0ca595fe047 Cr-Commit-Position: refs/heads/master@{#28045} https://codereview.chromium.org/1092353002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this messag

[v8-dev] Re: [strong] Disallow implicit casts in subtraction (issue 1092353002 by conr...@chromium.org)

2015-04-24 Thread commit-bot
Committed patchset #9 (id:340001) https://codereview.chromium.org/1092353002/ -- -- 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] Disallow implicit casts in subtraction (issue 1092353002 by conr...@chromium.org)

2015-04-24 Thread commit-bot
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1092353002/340001 https://codereview.chromium.org/1092353002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subsc

[v8-dev] Re: [strong] Disallow implicit casts in subtraction (issue 1092353002 by conr...@chromium.org)

2015-04-24 Thread conradw
Renamed https://codereview.chromium.org/1092353002/diff/320001/test/cctest/compiler/test-js-typed-lowering.cc File test/cctest/compiler/test-js-typed-lowering.cc (right): https://codereview.chromium.org/1092353002/diff/320001/test/cctest/compiler/test-js-typed-lowering.cc#newcode234 test/cctest

[v8-dev] Re: [strong] Disallow implicit casts in subtraction (issue 1092353002 by conr...@chromium.org)

2015-04-24 Thread mstarzinger
LGTM. The CL description talks about "casts", I think it it would make more sense to call it "conversion". https://codereview.chromium.org/1092353002/diff/320001/test/cctest/compiler/test-js-typed-lowering.cc File test/cctest/compiler/test-js-typed-lowering.cc (right): https://codereview.chromi

[v8-dev] Re: [strong] Disallow implicit casts in subtraction (issue 1092353002 by conr...@chromium.org)

2015-04-24 Thread conradw
https://codereview.chromium.org/1092353002/diff/260001/src/compiler/js-operator.h File src/compiler/js-operator.h (right): https://codereview.chromium.org/1092353002/diff/260001/src/compiler/js-operator.h#newcode236 src/compiler/js-operator.h:236: const Operator* UnaryNot(LanguageMode language_m

[v8-dev] Re: [strong] Disallow implicit casts in subtraction (issue 1092353002 by conr...@chromium.org)

2015-04-24 Thread mstarzinger
https://codereview.chromium.org/1092353002/diff/31/src/compiler/js-operator.h File src/compiler/js-operator.h (right): https://codereview.chromium.org/1092353002/diff/31/src/compiler/js-operator.h#newcode236 src/compiler/js-operator.h:236: const Operator* UnaryNot(LanguageMode language_m

[v8-dev] Re: [strong] Disallow implicit casts in subtraction (issue 1092353002 by conr...@chromium.org)

2015-04-24 Thread rossberg
https://codereview.chromium.org/1092353002/diff/260001/src/compiler/js-operator.h File src/compiler/js-operator.h (right): https://codereview.chromium.org/1092353002/diff/260001/src/compiler/js-operator.h#newcode236 src/compiler/js-operator.h:236: const Operator* UnaryNot(LanguageMode language_m

[v8-dev] Re: [strong] Disallow implicit casts in subtraction (issue 1092353002 by conr...@chromium.org)

2015-04-24 Thread conradw
@mstarzinger: everything looks ok? https://codereview.chromium.org/1092353002/diff/280001/src/ic/ic.cc File src/ic/ic.cc (right): https://codereview.chromium.org/1092353002/diff/280001/src/ic/ic.cc#newcode2811 src/ic/ic.cc:2811: switch (op) { On 2015/04/24 09:50:20, rossberg wrote: Nit: to com

[v8-dev] Re: [strong] Disallow implicit casts in subtraction (issue 1092353002 by conr...@chromium.org)

2015-04-24 Thread rossberg
LGTM modulo nits https://codereview.chromium.org/1092353002/diff/280001/src/ic/ic.cc File src/ic/ic.cc (right): https://codereview.chromium.org/1092353002/diff/280001/src/ic/ic.cc#newcode2811 src/ic/ic.cc:2811: switch (op) { Nit: to compress a little, can you put the returns on the same line as

[v8-dev] Re: [strong] Disallow implicit casts in subtraction (issue 1092353002 by conr...@chromium.org)

2015-04-23 Thread conradw
https://codereview.chromium.org/1092353002/diff/240001/src/compiler/js-operator.cc File src/compiler/js-operator.cc (right): https://codereview.chromium.org/1092353002/diff/240001/src/compiler/js-operator.cc#newcode233 src/compiler/js-operator.cc:233: #define CACHED_OP_LIST_WITH_STRONG(V)

[v8-dev] Re: [strong] Disallow implicit casts in subtraction (issue 1092353002 by conr...@chromium.org)

2015-04-23 Thread rossberg
https://codereview.chromium.org/1092353002/diff/240001/test/unittests/compiler/js-operator-unittest.cc File test/unittests/compiler/js-operator-unittest.cc (right): https://codereview.chromium.org/1092353002/diff/240001/test/unittests/compiler/js-operator-unittest.cc#newcode33 test/unittests/com

[v8-dev] Re: [strong] Disallow implicit casts in subtraction (issue 1092353002 by conr...@chromium.org)

2015-04-23 Thread mstarzinger
https://codereview.chromium.org/1092353002/diff/240001/src/compiler/js-operator.cc File src/compiler/js-operator.cc (right): https://codereview.chromium.org/1092353002/diff/240001/src/compiler/js-operator.cc#newcode233 src/compiler/js-operator.cc:233: #define CACHED_OP_LIST_WITH_STRONG(V)

[v8-dev] Re: [strong] Disallow implicit casts in subtraction (issue 1092353002 by conr...@chromium.org)

2015-04-23 Thread conradw
https://codereview.chromium.org/1092353002/diff/240001/src/code-stubs.h File src/code-stubs.h (right): https://codereview.chromium.org/1092353002/diff/240001/src/code-stubs.h#newcode1253 src/code-stubs.h:1253: BinaryOpICState state(isolate, op, language_mode); On 2015/04/23 13:29:37, rossberg wr

[v8-dev] Re: [strong] Disallow implicit casts in subtraction (issue 1092353002 by conr...@chromium.org)

2015-04-23 Thread mstarzinger
https://codereview.chromium.org/1092353002/diff/240001/src/compiler/ast-graph-builder.cc File src/compiler/ast-graph-builder.cc (right): https://codereview.chromium.org/1092353002/diff/240001/src/compiler/ast-graph-builder.cc#newcode1272 src/compiler/ast-graph-builder.cc:1272: Node* exit_cond =

[v8-dev] Re: [strong] Disallow implicit casts in subtraction (issue 1092353002 by conr...@chromium.org)

2015-04-23 Thread rossberg
Looks pretty good! Adding mstarzinger to sign off the TF changes. Sven, see my first comment regarding the language mode as a key to the BinaryICOpState -- I think that would likely cause unnecessary regressions. The question is at which level to do the mode->bool projection; would you prefer

[v8-dev] Re: [strong] Disallow implicit casts in subtraction (issue 1092353002 by conr...@chromium.org)

2015-04-22 Thread conradw
PTAL I've got rid of the previous patchsets since this attempt is mostly a new approach, the diffs just looked a bit silly. I implemented the bool strong -> LanguageMode change as suggested. https://codereview.chromium.org/1092353002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://gr

[v8-dev] Re: [strong] Disallow implicit casts in subtraction (issue 1092353002 by conr...@chromium.org)

2015-04-21 Thread svenpanne
Tiny DBC... https://codereview.chromium.org/1092353002/diff/40001/src/code-stubs.h File src/code-stubs.h (right): https://codereview.chromium.org/1092353002/diff/40001/src/code-stubs.h#newcode1251 src/code-stubs.h:1251: BinaryOpICStub(Isolate* isolate, Token::Value op, bool strong) Boolean para