http://codereview.chromium.org/8242002/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):
http://codereview.chromium.org/8242002/diff/1/src/hydrogen.cc#newcode5793
src/hydrogen.cc:5793: void
HGraphBuilder::HandleLiteralCompareTypeof(CompareOperation* expr,
On 2011/10/13 13:11:59, Kevin Millikin wrote:
I'd prefer to use more specific types here. The casts below will
always succeed
in this change, but it needs nonlocal reasoning to see that (and
relies on the
assertion to preserve it in the face of changes to the helper
functions).
void HGraphBuilder::HandleLiteralCompareTypeof(CompareOperation* expr,
HTypeof* typeof_expr,
Handle<String> blah) {
...
Done.
http://codereview.chromium.org/8242002/diff/1/src/hydrogen.cc#newcode5797
src/hydrogen.cc:5797: Handle<String> blah =
Handle<String>::cast(HConstant::cast(check)->handle());
On 2011/10/13 13:11:59, Kevin Millikin wrote:
There must be a better name.
hurz? (http://www.youtube.com/watch?v=RAx0P-8n5K4)
gnlpfth? (http://www.youtube.com/watch?v=olGad2Uhnwc) ;-)
Luckily, we don't need this variable anymore.
http://codereview.chromium.org/8242002/diff/1/src/hydrogen.cc#newcode5801
src/hydrogen.cc:5801: typeof_expr->DeleteAndReplaceWith(instr);
On 2011/10/13 13:11:59, Kevin Millikin wrote:
I don't think this is right. instr is a control instruction, and I
think we're
relying on having no uses for those. It should be the case that there
is no use
of typeof_expr, so you can replace it with NULL.
An alternative that I like a bit better, because it covers more cases,
is to
make HTypeof::Canonicalize() return NULL if there are no uses
(provided we
convince ourselves that typeof cannot have side effects).
I'd like to go for the first alternative (replacing with NULL) for now,
because it is a more local change of the CL. I'll have a look at the
Canonicalize() alternative later and submit a different CL if it makes
things clearer.
http://codereview.chromium.org/8242002/diff/1/src/hydrogen.cc#newcode5829
src/hydrogen.cc:5829: *typeof_expr = left;
On 2011/10/13 13:11:59, Kevin Millikin wrote:
If you take my suggestion above, this is where you'd cast:
*typeof_expr = HTypeof::cast(left);
*check = Handle<String>::cast(HConstant::cast(right)->handle());
Done.
http://codereview.chromium.org/8242002/diff/1/src/hydrogen.cc#newcode5891
src/hydrogen.cc:5891: HValue* typeof_expr = NULL;
On 2011/10/13 13:11:59, Kevin Millikin wrote:
And
HTypeof* typeof_expr = NULL;
Handle<String> check;
Done.
http://codereview.chromium.org/8242002/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
