https://codereview.chromium.org/14990014/diff/3001/src/ast.cc
File src/ast.cc (right):

https://codereview.chromium.org/14990014/diff/3001/src/ast.cc#newcode421
src/ast.cc:421: for_in_type_ =
static_cast<ForInType>(oracle->ForInType(this));
On 2013/05/16 11:58:05, Sven Panne wrote:
Hmmm, where should types like ForInType live in the long run? I don't
have a
good idea yet, casts are :-P, circular dependencies are even more :-P

I don't have a good idea either. Probably will have to be some special
type domains on the side of 'real' types.

https://codereview.chromium.org/14990014/diff/3001/src/ast.h
File src/ast.h (right):

https://codereview.chromium.org/14990014/diff/3001/src/ast.h#newcode376
src/ast.h:376: byte to_boolean_types() const { return to_boolean_types_;
}
On 2013/05/16 11:58:05, Sven Panne wrote:
Do we still have problems with #include cycles, so we have to use
"byte" here?
If not, the real type would be nicer.

Yes. But once the type representation has been unified, this problem
should have disappeared (because the low-level feedback types will be
encapsulated in the oracle).

https://codereview.chromium.org/14990014/diff/3001/src/ast.h#newcode896
src/ast.h:896: ForInType for_in_type_;
On 2013/05/16 11:58:05, Sven Panne wrote:
Can we keep the fields and methods in a common order, e.g. syntactical
parts
(i.e. the "real" AST children), then type annotations, and finally
bailout/feedback IDs? I don't care too much about the actual order,
more about
the consistency. Separating the parts by empty lines or comments might
be nice,
too.

Done here and in a number of other places.

https://codereview.chromium.org/14990014/diff/3001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/14990014/diff/3001/src/hydrogen.cc#newcode9783
src/hydrogen.cc:9783: ASSERT(feedback->IsSmi());
On 2013/05/16 11:58:05, Sven Panne wrote:
Looks a bit redundant now...

Indeed :)

https://codereview.chromium.org/14990014/diff/3001/src/typing.cc
File src/typing.cc (right):

https://codereview.chromium.org/14990014/diff/3001/src/typing.cc#newcode84
src/typing.cc:84: CHECK_ALIVE(VisitStatements(stmt->statements()));
On 2013/05/16 11:58:05, Sven Panne wrote:
Hmmm, do we really need a CHECK_ALIVE here? I thought we need this
only for
early return. Anyway, can't we somehow have this centrally in
AstVisitor::Visit?
There is already a check, but it is not 100% clear to me why that
isn't enough.
Maybe monad by hand?

You have to check at every level to make failure abort, right? And yes,
it's a manual Maybe monad.

https://codereview.chromium.org/14990014/diff/3001/src/typing.h
File src/typing.h (right):

https://codereview.chromium.org/14990014/diff/3001/src/typing.h#newcode48
src/typing.h:48: void* operator new(size_t size, Zone* zone) {
On 2013/05/16 11:58:05, Sven Panne wrote:
What is the reason for the new/delete definitions here? If they are
really
needed, a comment is appropriate. Furthermore, this is effectively
making
AstTyper a ZoneObject (why?). Is it just avoiding multiple
inheritance?

*sigh* In general, we should really templatize things more, this would
just get
a ZoneAllocationPolicy then. We are extremely ad-hoc in such things.
:-(

This is simply analogous to the graph builder. Not sure I understand
your question, why would it not be zone-allocated?

https://codereview.chromium.org/14990014/

--
--
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