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

Issue 7918012: Unify the handling of comparinsons against null and undefined. (Closed)

Created:
9 years, 3 months ago by Sven Panne
Modified:
9 years, 3 months ago
Reviewers:
fschneider, danno
CC:
v8-dev
Visibility:
Public.

Description

Unify the handling of comparinsons against null and undefined. Although this patch is not small, most parts of it are rather mechanical: * First of all, the concept of a 'nil-like' value is introduced, which can be null or undefined. They are treated symmetrically regarding comparisons, so it makes sense to handle them in a uniform manner. It is a mystery why JavaScript defines two of those beasts, when even *one* is a design wart... * Extended and renamed a few things which now handle undefined in addition to null. * Made the parts of the full code generator and the hydrogen generation which deal with comparisons a bit more similar regarding their handling of special cases. * Refactored the syntactical detection of special cases for comparisons, hopefully making them a bit more readable and less copy-n-paste-oriented. Things like this should really be a one-liner in any sane programming language... :-P * Cut down the length of the argument lists of a few functions to something less insane, making them more easily understandable locally. This involves minor code duplication, but this was a good tradeoff and can be remedied later if necessary. * Replaced some boolean arguments with more readable enums. * Fixed a TODO: Values which are definitely a Smi or unboxed can never be equal to null or undefined. Committed: http://code.google.com/p/v8/source/detail?r=9323

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+358 lines, -322 lines) Patch
M src/arm/full-codegen-arm.cc View 5 chunks +34 lines, -33 lines 0 comments Download
M src/arm/lithium-arm.h View 2 chunks +7 lines, -6 lines 0 comments Download
M src/arm/lithium-arm.cc View 2 chunks +5 lines, -4 lines 0 comments Download
M src/arm/lithium-codegen-arm.cc View 1 chunk +18 lines, -8 lines 0 comments Download
M src/ast.h View 2 chunks +8 lines, -0 lines 0 comments Download
M src/ast.cc View 1 chunk +49 lines, -49 lines 0 comments Download
M src/full-codegen.h View 1 chunk +6 lines, -21 lines 0 comments Download
M src/full-codegen.cc View 1 chunk +8 lines, -12 lines 0 comments Download
M src/hydrogen.h View 1 chunk +6 lines, -6 lines 0 comments Download
M src/hydrogen.cc View 3 chunks +36 lines, -38 lines 0 comments Download
M src/hydrogen-instructions.h View 2 chunks +9 lines, -7 lines 0 comments Download
M src/ia32/full-codegen-ia32.cc View 5 chunks +35 lines, -34 lines 0 comments Download
M src/ia32/lithium-codegen-ia32.cc View 1 chunk +18 lines, -8 lines 0 comments Download
M src/ia32/lithium-ia32.h View 2 chunks +7 lines, -6 lines 0 comments Download
M src/ia32/lithium-ia32.cc View 2 chunks +6 lines, -5 lines 0 comments Download
M src/mips/full-codegen-mips.cc View 5 chunks +34 lines, -33 lines 0 comments Download
M src/token.h View 1 chunk +4 lines, -0 lines 0 comments Download
M src/v8.h View 1 chunk +9 lines, -0 lines 4 comments Download
M src/x64/full-codegen-x64.cc View 5 chunks +34 lines, -32 lines 0 comments Download
M src/x64/lithium-codegen-x64.cc View 1 chunk +12 lines, -9 lines 0 comments Download
M src/x64/lithium-x64.h View 2 chunks +7 lines, -6 lines 0 comments Download
M src/x64/lithium-x64.cc View 2 chunks +6 lines, -5 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Sven Panne
9 years, 3 months ago (2011-09-16 08:08:02 UTC) #1
danno
LGTM with nits. http://codereview.chromium.org/7918012/diff/1/src/v8.h File src/v8.h (right): http://codereview.chromium.org/7918012/diff/1/src/v8.h#newcode128 src/v8.h:128: // JavaScript defines two kinds of ...
9 years, 3 months ago (2011-09-19 14:38:50 UTC) #2
Sven Panne
9 years, 3 months ago (2011-09-19 14:45:57 UTC) #3
http://codereview.chromium.org/7918012/diff/1/src/v8.h
File src/v8.h (right):

http://codereview.chromium.org/7918012/diff/1/src/v8.h#newcode128
src/v8.h:128: // JavaScript defines two kinds of 'nil', for whatever reasons...
On 2011/09/19 14:38:50, danno wrote:
> no personalized comment (no "for whatever reasons") :-)

Done.

http://codereview.chromium.org/7918012/diff/1/src/v8.h#newcode132
src/v8.h:132: // JavaScript defines two kinds of equality, again, for whatever
reasons...
On 2011/09/19 14:38:50, danno wrote:
> here, too.

Done.

Powered by Google App Engine
This is Rietveld 408576698