Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(129)

Issue 2835233006: VM: [Kernel] convert null checks to StrictCompare (Closed)

Created:
3 years, 7 months ago by Dmitry Olshansky
Modified:
3 years, 7 months ago
CC:
reviews_dartlang.org, vm-dev_dartlang.org
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Patch Set 1 #

Total comments: 1

Patch Set 2 : VM: [Kernel] convert null checks to StrictCompare #

Total comments: 1

Patch Set 3 : VM: [Kernel] convert null checks to StrictCompare #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+30 lines, -0 lines) Patch
M runtime/vm/kernel_to_il.h View 1 1 chunk +3 lines, -0 lines 0 comments Download
M runtime/vm/kernel_to_il.cc View 1 2 3 chunks +27 lines, -0 lines 2 comments Download

Messages

Total messages: 10 (2 generated)
Dmitry Olshansky
3 years, 7 months ago (2017-04-25 17:09:00 UTC) #2
Vyacheslav Egorov (Google)
https://codereview.chromium.org/2835233006/diff/1/runtime/vm/kernel_to_il.cc File runtime/vm/kernel_to_il.cc (right): https://codereview.chromium.org/2835233006/diff/1/runtime/vm/kernel_to_il.cc#newcode4969 runtime/vm/kernel_to_il.cc:4969: fragment_ = ToStrictNullCheck(this, token_kind, node); I don't think this ...
3 years, 7 months ago (2017-04-27 08:51:34 UTC) #3
Dmitry Olshansky
On 2017/04/27 08:51:34, Vyacheslav Egorov (Google) wrote: > https://codereview.chromium.org/2835233006/diff/1/runtime/vm/kernel_to_il.cc > File runtime/vm/kernel_to_il.cc (right): > > ...
3 years, 7 months ago (2017-04-27 10:30:43 UTC) #4
Vyacheslav Egorov (Google)
lgtm https://codereview.chromium.org/2835233006/diff/20001/runtime/vm/kernel_to_il.cc File runtime/vm/kernel_to_il.cc (right): https://codereview.chromium.org/2835233006/diff/20001/runtime/vm/kernel_to_il.cc#newcode4930 runtime/vm/kernel_to_il.cc:4930: if (token_kind == Token::kEQ || token_kind == Token::kNE) ...
3 years, 7 months ago (2017-04-27 10:48:18 UTC) #5
Dmitry Olshansky
Committed patchset #3 (id:40001) manually as e71c21b3b14f180f69cce856a2ff86a913b66b1f (presubmit successful).
3 years, 7 months ago (2017-04-27 11:22:50 UTC) #7
kustermann
https://codereview.chromium.org/2835233006/diff/40001/runtime/vm/kernel_to_il.cc File runtime/vm/kernel_to_il.cc (right): https://codereview.chromium.org/2835233006/diff/40001/runtime/vm/kernel_to_il.cc#newcode4941 runtime/vm/kernel_to_il.cc:4941: /*number_check = */ true); Since either [left] or [right] ...
3 years, 7 months ago (2017-04-29 20:37:24 UTC) #8
Dmitry Olshansky
On 2017/04/29 20:37:24, kustermann wrote: > https://codereview.chromium.org/2835233006/diff/40001/runtime/vm/kernel_to_il.cc > File runtime/vm/kernel_to_il.cc (right): > > https://codereview.chromium.org/2835233006/diff/40001/runtime/vm/kernel_to_il.cc#newcode4941 > ...
3 years, 7 months ago (2017-04-29 20:44:42 UTC) #9
Vyacheslav Egorov (Google)
3 years, 7 months ago (2017-04-30 04:50:25 UTC) #10
Message was sent while issue was closed.
https://codereview.chromium.org/2835233006/diff/40001/runtime/vm/kernel_to_il.cc
File runtime/vm/kernel_to_il.cc (right):

https://codereview.chromium.org/2835233006/diff/40001/runtime/vm/kernel_to_il...
runtime/vm/kernel_to_il.cc:4941: /*number_check = */ true);
On 2017/04/29 20:37:24, kustermann wrote:
> Since either [left] or [right] is null and null is a singleton, do we need a
> number check here?

Yes, unfortunately, because it acts as a debugger patch point.

Powered by Google App Engine
This is Rietveld 408576698