On Thu, Sep 11, 2008 at 4:11 PM,  <[EMAIL PROTECTED]> wrote:
> Hi Dean
>
> Great first step to move all flags to one file.
> However, it would be perfect if you could eliminate the flags.defs.
>
> Otherwise, LGTM,
>  Lars
>
>
> http://codereview.chromium.org/1935/diff/53/271
> File src/flags.defs (right):
>
> http://codereview.chromium.org/1935/diff/53/271#newcode1
> Line 1: // Copyright 2008 the V8 project authors. All rights reserved.
> I understand the reason for the state based include but we should reduce
> flags to be in release or in release+debug.
> Since most flags is compiled away in release mode, I think this is OK.
> If you make this change flags can be defined in ONE giant macro.
> This will also eliminate the need for flags.def.

Please see v8-counters.h to why this won't / doesn't work well.

I agree it would be nice to have less "modes", but maybe that's
something we could refactor later?  I wanted to make this change,
leaving the flags system working identically to how it works now.
Changing both at the same time is too many moving pieces, this was a
pain enough as is...

>
> http://codereview.chromium.org/1935/diff/53/272
> File src/flags.h (right):
>
> http://codereview.chromium.org/1935/diff/53/272#newcode35
> Line 35: #define FLAG_MODE_DECLARE
> I don't like the defs file extension. Please use .h as extension. After
> all, it is a C++ header file.

This file won't lint, and it's not really a header in the normal
sense.  Making it a .h will break the build unless we make a lint
exception.  It's up to you I guess.

>
> http://codereview.chromium.org/1935
>

--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to