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()));
On 2015/02/11 12:34:10, rossberg wrote:
Why this?
I think we should not fail to give a minimum if we have some funny
bitset, such as Signed32 / UntaggedPointer.
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.
On 2015/02/11 12:34:10, rossberg wrote:
Nit: add "..., which always is a bitset."
Done.
https://codereview.chromium.org/904863002/diff/40001/src/types.cc#newcode167
src/types.cc:167: // Other elements only contribute the semantic part.
On 2015/02/11 12:34:10, rossberg wrote:
Nit: either "contribute to the" or "contribute their"
Done.
https://codereview.chromium.org/904863002/diff/40001/src/types.cc#newcode519
src/types.cc:519: return BitsetType::Is(SEMANTIC(this->BitsetLub()),
that->AsBitset());
On 2015/02/11 12:34:10, rossberg wrote:
Don't you need SEMANTIC on 'that' as well?
I do not think so. That is purely because the representation of the
right hand side cannot possibly affect the result (left hand side
representation is empty, so it is always subtype of the right hand side
representation). I agree that this is not quite obvious, and perhaps a
premature optimization.
https://codereview.chromium.org/904863002/diff/40001/src/types.cc#newcode522
src/types.cc:522: return BitsetType::Is(SEMANTIC(this->AsBitset()),
that->BitsetGlb());
On 2015/02/11 12:34:10, rossberg wrote:
Similarly here?
See above.
https://codereview.chromium.org/904863002/diff/40001/src/types.cc#newcode684
src/types.cc:684: // (even when the first element is not a bitset).
On 2015/02/11 12:34:10, rossberg wrote:
This line is obsolete now.
Done.
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.,
On 2015/02/11 12:34:10, rossberg wrote:
"should not make"?
Done.
https://codereview.chromium.org/904863002/diff/40001/src/types.cc#newcode995
src/types.cc:995: result->Set(size++, range);
On 2015/02/11 12:34:10, rossberg wrote:
Nit: single line
Done.
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();
On 2015/02/11 12:34:11, rossberg wrote:
Nit: drop the "Get"
Done.
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
On 2015/02/11 12:34:11, rossberg wrote:
Update comment
Done.
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());
On 2015/02/11 12:34:11, rossberg wrote:
Can we be more precise here? E.g.,
CheckEqual(T.Intersect(T.ObjectClass, T.Array)->Semantic(), T.None);
Done.
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) {
On 2015/02/11 12:34:11, rossberg wrote:
The Type class should probably just have two methods you can forward
these to:
TypeHandle Semantic()
TypeHandle Representation()
Done.
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.
