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

Issue 18901003: Compute expected type for variables (Closed)

Created:
7 years, 5 months ago by rossberg
Modified:
5 years ago
Reviewers:
Jakob Kummerow
CC:
v8-dev
Visibility:
Public.

Description

Compute expected type for variables That is, the intersection of all types it will have to be coerced to. R=jkummerow@chromium.org BUG=

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+146 lines, -65 lines) Patch
M src/typing.h View 1 chunk +1 line, -0 lines 0 comments Download
M src/typing.cc View 27 chunks +139 lines, -65 lines 3 comments Download
M src/variables.h View 3 chunks +5 lines, -0 lines 1 comment Download
M src/variables.cc View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 3 (1 generated)
rossberg
7 years, 5 months ago (2013-07-09 13:41:01 UTC) #1
Jakob Kummerow
7 years, 5 months ago (2013-07-09 16:01:50 UTC) #2
https://codereview.chromium.org/18901003/diff/1/src/typing.cc
File src/typing.cc (right):

https://codereview.chromium.org/18901003/diff/1/src/typing.cc#newcode281
src/typing.cc:281: Type::Intersect(var->expected_type(), expected_type_),
isolate_));
As discussed offline, I still think a Union would make more sense here.

https://codereview.chromium.org/18901003/diff/1/src/typing.cc#newcode355
src/typing.cc:355: RECURSE(Visit(expr->value()));
Shouldn't this be RECURSE_EXPR(_, Type::Any())?

https://codereview.chromium.org/18901003/diff/1/src/typing.cc#newcode384
src/typing.cc:384: RECURSE_EXPR(Visit(expr->key()), Type::Name());
I'm pretty sure that the key can also be a number.

https://codereview.chromium.org/18901003/diff/1/src/variables.h
File src/variables.h (right):

https://codereview.chromium.org/18901003/diff/1/src/variables.h#newcode169
src/variables.h:169: Handle<Type> expected_type_;  // Intersection of all reads
nit: trailing full stop. And, uhm, "Union" ;-)

Powered by Google App Engine
This is Rietveld 408576698