[webkit-reviews] review denied: [Bug 3576] roll in support for "const" keyword from KDE tree : [Attachment 2513] Patch for const support, cleaner fix than KDE version IMO

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Mon Jun 20 19:08:48 PDT 2005


Darin Adler <darin at apple.com> has denied Maciej Stachowiak <mjs at apple.com>'s
request for review:
Bug 3576: roll in support for "const" keyword from KDE tree
http://bugzilla.opendarwin.org/show_bug.cgi?id=3576

Attachment 2513: Patch for const support, cleaner fix than KDE version IMO
http://bugzilla.opendarwin.org/attachment.cgi?id=2513&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
Looks, good. Here are my comments:

One hanging "-see also test case" seen in JavaScriptCore/ChangeLog. You
probably meant to reference the test case in WebCore.

VarDeclNode::processVarDecls seems to be different from before. It's setting
the Internal bit on the property it's setting, and also sometimes not setting
the DontDelete flag. Is that a correct change? If so, then maybe we should have
a test case that depends on it.

Strange formatting in the line with the "already declared ?" comment -- it adds
spaces in a format we don't usually use.

Missing space after "exec," in variable.put line.

Is there a reason for the change in formatting in the VarStatementNode
constructor?

There are tabs in the ChangeLog entries -- should be spaces.

Does this cause problems with use of the identifier "const" as, say, the name
of a variable, function, or property?



More information about the webkit-reviews mailing list