Just a few more quick remarks. I still don't understand why we suddenly need 5
things per node (AllocateBailoutIdRange, InitializeBaseBailoutId, base_id,
leaf_base_id, kBailoutIdCount) instead of the 2 before (base_id, num_ids), so
please don't land it yet.


https://codereview.chromium.org/636403003/diff/1/src/ast-numbering.cc
File src/ast-numbering.cc (right):

https://codereview.chromium.org/636403003/diff/1/src/ast-numbering.cc#newcode24
src/ast-numbering.cc:24: #define DEFINE_VISIT(type)                  \
I don't like this 2nd order macro horror, although I understand the
motivation behind it: Separating the recursion scheme from the actual
logic per node.

As it is, it obfuscates things more than it helps, so just directly
implement all those VisitFoo directly (where Foo is an AST node type).
This takes 10 minutes more to write and saves 10h reading the code
later.

https://codereview.chromium.org/636403003/diff/1/src/ast-numbering.cc#newcode250
src/ast-numbering.cc:250: bool AstNumbering::Renumber(CompilationInfo*
info) {
This is coupled too tightly: Just pass in the FunctionLiteral. It's a
pity that we need the zone in addition (only to get the Isolate and that
only for the stack check :-P ), but that's just the 2nd parameter then.

https://codereview.chromium.org/636403003/diff/1/src/ast-numbering.cc#newcode256
src/ast-numbering.cc:256: if (visitor.HasStackOverflow()) return false;
return !visitor.HasStackOverflow();

https://codereview.chromium.org/636403003/

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

Reply via email to