[webkit-reviews] review denied: [Bug 14868] Import variable lookup
optimizations from KJS : [Attachment 15815] Proposed patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sat Aug 4 17:07:35 PDT 2007
Oliver Hunt <oliver at apple.com> has denied Cameron Zwarich (cpst)
<cwzwarich at uwaterloo.ca>'s request for review:
Bug 14868: Import variable lookup optimizations from KJS
http://bugs.webkit.org/show_bug.cgi?id=14868
Attachment 15815: Proposed patch
http://bugs.webkit.org/attachment.cgi?id=15815&action=edit
------- Additional Comments from Oliver Hunt <oliver at apple.com>
Need an extra space for indenting the comment at
+ if (!body->builtSymbolList()) {
+ // now handle other locals (functions, variables)
+ body->processDecls(&newExec);
I'm less keen on + //### Document this, ugh.
function.h should also probably have you copyright info now
ActivationImp
putLocal -> replace assert with ASSERT
putLocalChecked -> return on a new line
isLocalReadOnly -> i'm not sure but i believe VS will want
!!(_locals[propertyID].attr & ReadOnly)
VarAccessNode
optimizeLocalAccess -> no braces for "} else {" (and the closing '}')
NonLocalVarAccessNode
evaluateReference -> s/assert/ASSERT/
nodes.h should possibly gain your copyright
there are quite a few other places where assert is used instead of ASSERT
There are quite a few portions of this patch that look fairly trivial to make
independent of this major patch -- for instance the [int] optimisation seems
completely distinct from the improved scope lookup
More information about the webkit-reviews
mailing list