[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