Issue 12217136: Fix NegateCompareOp and InvertCompareOp

Issue 12217136: Fix NegateCompareOp and InvertCompareOp (Closed)

Created:
7 years, 10 months ago by Jakob Kummerow

Modified:
7 years, 10 months ago

Reviewers:
Sven Panne, Michael Starzinger

CC:
v8-dev

Base URL:
https://v8.googlecode.com/svn/branches/bleeding_edge

Visibility:
Public.

More Reviews

Description

Fix NegateCompareOp and InvertCompareOp BUG=v8:2537 Committed: http://code.google.com/p/v8/source/detail?r=13658

Patch Set 2 : addressed comments #

Created: 7 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+21 lines, -12 lines)			Patch
M	src/hydrogen.cc	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	src/token.h	View	1	1 chunk	+8 lines, -4 lines	0 comments	Download
A +	test/mjsunit/regress/regress-2537.js	View		1 chunk	+12 lines, -7 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

Michael Starzinger

LGTM. Nice catch. One comment though. https://codereview.chromium.org/12217136/diff/1/src/token.h File src/token.h (right): https://codereview.chromium.org/12217136/diff/1/src/token.h#newcode239 src/token.h:239: return op; Same ...

7 years, 10 months ago (2013-02-13 11:01:18 UTC) #2

Sven Panne

https://codereview.chromium.org/12217136/diff/1/src/token.h File src/token.h (right): https://codereview.chromium.org/12217136/diff/1/src/token.h#newcode243 src/token.h:243: static Value InvertCompareOp(Value op) { DBC: Can we rename ...

7 years, 10 months ago (2013-02-13 11:59:30 UTC) #3

Jakob Kummerow

7 years, 10 months ago (2013-02-13 14:35:43 UTC) #4

Addressed comments: renamed the function and marked the default case as
UNREACHABLE.

https://codereview.chromium.org/12217136/diff/1/src/token.h
File src/token.h (right):

https://codereview.chromium.org/12217136/diff/1/src/token.h#newcode239
src/token.h:239: return op;
On 2013/02/13 11:01:18, Michael Starzinger wrote:
> Same argumentation as below applies.

Done.

https://codereview.chromium.org/12217136/diff/1/src/token.h#newcode255
src/token.h:255: return op;
On 2013/02/13 11:01:18, Michael Starzinger wrote:
> The "IN" and the "INSTANCEOF" operation cannot really be inverted I think. So
> calling InvertCompareOp() on them would point to a bug I guess. Can we hence
> change the default branch to assert that it's UNREACHABLE()?

Done.

Expand Messages | Collapse Messages