https://codereview.chromium.org/17842004/diff/11001/src/typing.cc
File src/typing.cc (right):

https://codereview.chromium.org/17842004/diff/11001/src/typing.cc#newcode251
src/typing.cc:251: MergeLowerType(expr, Type::Union(
On 2013/06/26 15:09:23, Jakob wrote:
As discussed offline, inferring the lower bound as the union may be a
little too
pessimistic. Also, it should be consistent with OR and AND below.

Actually, the correct thing to use here is intersection.

https://codereview.chromium.org/17842004/diff/11001/src/typing.cc#newcode319
src/typing.cc:319: expr->RecordTypeFeedback(oracle(), zone());
On 2013/06/26 15:09:23, Jakob wrote:
nit: {} while you're here

Done.

https://codereview.chromium.org/17842004/diff/11001/src/typing.cc#newcode326
src/typing.cc:326: expr->RecordTypeFeedback(oracle(), zone());
On 2013/06/26 15:09:23, Jakob wrote:
nit: {}

Done.

https://codereview.chromium.org/17842004/diff/11001/src/typing.cc#newcode346
src/typing.cc:346: // Lower type is None already
On 2013/06/26 15:09:23, Jakob wrote:
nit: trailing full stop please.

Done.

https://codereview.chromium.org/17842004/diff/11001/src/typing.cc#newcode413
src/typing.cc:413: if (expr->op() == Token::NOT) {
On 2013/06/26 15:09:23, Jakob wrote:
Combine this with the switch below maybe? If you want to keep calls to
RecordFooTypeFeedback separate from the logic related to default
bounds that's
fine too.

Yeah, I'd like to keep it separate. (And we should perhaps get rid of
the conditional.)

https://codereview.chromium.org/17842004/diff/11001/src/typing.cc#newcode457
src/typing.cc:457: if (expr->is_prefix()) {
On 2013/06/26 15:09:23, Jakob wrote:
I don't think we need all this logic here. Regardless of prefix or
postfix, the
lower types is always Smi and the upper type is always Number.

Good catch. Done.

https://codereview.chromium.org/17842004/diff/11001/src/typing.cc#newcode501
src/typing.cc:501: // No lower type
On 2013/06/26 15:09:23, Jakob wrote:
nit: trailing full stop please.

Comment is obsolete now.

https://codereview.chromium.org/17842004/diff/11001/src/typing.cc#newcode513
src/typing.cc:513: case Token::SHR:
On 2013/06/26 15:09:23, Jakob wrote:
I think you're mixing up SHR and SAR here.

Done.

https://codereview.chromium.org/17842004/diff/11001/src/typing.cc#newcode522
src/typing.cc:522: // No unique lower type (could be Smi or string)
On 2013/06/26 15:09:23, Jakob wrote:
nit: trailing full stop please.

Done.

https://codereview.chromium.org/17842004/

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to