Drive by:

Please no scoped objects.  I would rather just see a wrapper around Visit
used for an expression in a test context.

To elaborate: the AST visitor itself could be spelled "unnecessary
bookkeeping" because it can't take extra arguments or return a result.  In
this case, I think what is really wanted is a function from expressions and
label pairs to the side effect of emitting code (of type 'void
(*)(Expression*, Label*, Label*)').  We should have a wrapper with that type
and *completely* hide the setting and resetting of internal state, rather
than a scoped object that partially hides it.

On Tue, Nov 10, 2009 at 4:35 AM, <[email protected]> wrote:

>
> LGTM
>
>
> http://codereview.chromium.org/371070/diff/1/3
> File src/fast-codegen.cc (right):
>
> http://codereview.chromium.org/371070/diff/1/3#newcode319
> Line 319: true_label_ = &body;
> We should probably have some scoped object that set and unset/reset both
> labels. This seems like unnecessary book-keeping.
>
> http://codereview.chromium.org/371070
>
> >
>

--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to