Great, I'll commit soon. Having a fun time merging all of the NewArray / Vector changes and flag additions/changes.
On Fri, Sep 12, 2008 at 10:20 AM, Lars Bak <[EMAIL PROTECTED]> wrote: > Hi Dean > > This is a first good step. Please commit you changes. > The changes I suggested can be added later. > LGTM > > Regards, > Lars > > On Thu, Sep 11, 2008 at 4:15 PM, Dean McNamee <[EMAIL PROTECTED]> wrote: >> 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 [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
