[Webkit-unassigned] [Bug 65839] CodeGenerator*.pm should support Web IDL Constructor attribute

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=65839





--- Comment #4 from Dominic Cooney <dominicc at chromium.org>  2011-08-14 18:30:20 PST ---
(From update of attachment 103879)
View in context: https://bugs.webkit.org/attachment.cgi?id=103879&action=review

This looks really good. Since this patch is pretty big we should work to get this in without expanding the scope any further yet.

You can edit the files in Source/WebCore/bindings/scripts/test/*.idl and run Tools/Scripts/run-bindings-tests to generate test output. There is a flag to reset the expected output too. This can be useful because it is possible to review the effect of changes to CodeGeneratorV8.pm; what effect metadata has.

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:1487
> +        push(@implContent, <<END);

Maybe just use a string literal?

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:1552
> +sub IsContainValue

Just call it ContainsValue

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:1558
> +    my @valueList = split(/\|/, $orSeparatedValueString);

Does the | mean "either" or "and". Maybe | is not the best choice of separator?

> Source/WebCore/bindings/scripts/IDLParser.pm:186
> +        }

I wish the IDL parser was recursive descent too; I think this is still quite readable though.

> Source/WebCore/css/WebKitCSSMatrix.idl:34
> +        ConstructorWith=CreateException|DOMObject

You don’t really need to do this | thing. You could have separate attributes like:

Constructor(…)
ConstructWith=ScriptExecutionContext
ConstructorSetsDOMWrapper
ConstructorRaisesException

This is verbose, but it is clearer. Also, I think "DOMObject" might only apply to V8, not JSC (not sure) so we could make that one V8ConstructorSetsDOMWrapper.

Do you think that there are any constructors that have different attributes in these dimensions (eg one overload that throws exceptions and one that doesn’t?)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.