[webkit-reviews] review granted: [Bug 67311] Add EnabledAtRuntime support for constants. : [Attachment 109302] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Sep 30 10:01:03 PDT 2011
Adam Barth <abarth at webkit.org> has granted Aaron Colwell
<acolwell at chromium.org>'s request for review:
Bug 67311: Add EnabledAtRuntime support for constants.
https://bugs.webkit.org/show_bug.cgi?id=67311
Attachment 109302: Patch
https://bugs.webkit.org/attachment.cgi?id=109302&action=review
------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=109302&action=review
> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:2113
> # FIXME: we need the static_cast here only because of one constant,
NodeFilter.idl
> # defines "const unsigned long SHOW_ALL = 0xFFFFFFFF". It would be
better if we
> # handled this here, and converted it to a -1 constant in the c++
output.
Can you move this comment inside the else branch? That's really where it
belongs.
> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:2253
> + static const BatchedConstant constData = {"${name}",
static_cast<signed int>(${value})};
> + batchConfigureConstants(desc, proto, &constData, 1);
Its too bad we don't get much of a batching savings here, but I don't see how
to avoid that. Maybe if we grouped by condition? That doesn't seem
worthwhile.
More information about the webkit-reviews
mailing list