Cool, mostly nits.
https://codereview.chromium.org/904863002/diff/20001/src/types.cc
File src/types.cc (right):
https://codereview.chromium.org/904863002/diff/20001/src/types.cc#newcode97
src/types.cc:97: DCHECK(this->SemanticIs(Number()));
Why this?
https://codereview.chromium.org/904863002/diff/40001/src/types.cc
File src/types.cc (right):
https://codereview.chromium.org/904863002/diff/40001/src/types.cc#newcode164
src/types.cc:164: // Take the representation from the first element.
Nit: add "..., which always is a bitset."
https://codereview.chromium.org/904863002/diff/40001/src/types.cc#newcode167
src/types.cc:167: // Other elements only contribute the semantic part.
Nit: either "contribute to the" or "contribute their"
https://codereview.chromium.org/904863002/diff/40001/src/types.cc#newcode519
src/types.cc:519: return BitsetType::Is(SEMANTIC(this->BitsetLub()),
that->AsBitset());
Don't you need SEMANTIC on 'that' as well?
https://codereview.chromium.org/904863002/diff/40001/src/types.cc#newcode522
src/types.cc:522: return BitsetType::Is(SEMANTIC(this->AsBitset()),
that->BitsetGlb());
Similarly here?
https://codereview.chromium.org/904863002/diff/40001/src/types.cc#newcode684
src/types.cc:684: // (even when the first element is not a bitset).
This line is obsolete now.
https://codereview.chromium.org/904863002/diff/40001/src/types.cc#newcode740
src/types.cc:740: // it should make any decisions based on
representations (i.e.,
"should not make"?
https://codereview.chromium.org/904863002/diff/40001/src/types.cc#newcode995
src/types.cc:995: result->Set(size++, range);
Nit: single line
https://codereview.chromium.org/904863002/diff/40001/src/types.h
File src/types.h (right):
https://codereview.chromium.org/904863002/diff/40001/src/types.h#newcode576
src/types.h:576: bitset GetRepresentation();
Nit: drop the "Get"
https://codereview.chromium.org/904863002/diff/40001/test/cctest/test-types.cc
File test/cctest/test-types.cc (right):
https://codereview.chromium.org/904863002/diff/40001/test/cctest/test-types.cc#newcode724
test/cctest/test-types.cc:724: // Rangification: If
T->Is(Range(-inf,+inf)) and !T->Is(None), then
Update comment
https://codereview.chromium.org/904863002/diff/40001/test/cctest/test-types.cc#newcode1761
test/cctest/test-types.cc:1761: CHECK(!T.Intersect(T.ObjectClass,
T.Array)->IsInhabited());
Can we be more precise here? E.g.,
CheckEqual(T.Intersect(T.ObjectClass, T.Array)->Semantic(), T.None);
https://codereview.chromium.org/904863002/diff/40001/test/cctest/types-fuzz.h
File test/cctest/types-fuzz.h (right):
https://codereview.chromium.org/904863002/diff/40001/test/cctest/types-fuzz.h#newcode230
test/cctest/types-fuzz.h:230: TypeHandle
ProjectRepresentation(TypeHandle t) {
The Type class should probably just have two methods you can forward
these to:
TypeHandle Semantic()
TypeHandle Representation()
https://codereview.chromium.org/904863002/
--
--
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/d/optout.
