[webkit-reviews] review denied: [Bug 52340] Look into possibilities of typedef in webkit idl files : [Attachment 188111] Patch

[bugzilla-daemon at webkit.org]

Kentaro Hara <haraken at chromium.org> has denied Andrey Adaikin
<aandrey at chromium.org>'s request for review:
Bug 52340: Look into possibilities of typedef in webkit idl files
https://bugs.webkit.org/show_bug.cgi?id=52340

Attachment 188111: Patch
https://bugs.webkit.org/attachment.cgi?id=188111&action=review

------- Additional Comments from Kentaro Hara <haraken at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=188111&action=review


Thanks for the patch!

> Source/WebCore/bindings/scripts/IDLParser.pm:361
> +		   if (exists $typedefs{$constant->type()}) {

Nit: type() => type

The same comments for other parts.

> Source/WebCore/bindings/scripts/IDLParser.pm:363
> +		      
$self->assertNoExtendedAttributesInTypedef($constant->type(), __LINE__);

Is this speced?

> Source/WebCore/bindings/scripts/IDLParser.pm:368
> +		   $self->applyTypedefsForSignature($attribute->signature());

Nit: signature() => signature

> Source/WebCore/bindings/scripts/IDLParser.pm:392
> +    my $type = $signature->type();
> +    $type =~ s/[\?\[\]]+$//g;
> +    my $typeSuffix = $signature->type();
> +    $typeSuffix =~ s/^[^\?\[\]]+//g;

How about simply replacing a typedefed-type with a real type during parsing,
instead of doing the replacement at the end of the parsing? By doing this, you
don't need tricky regular expressions to replace types. Simply, every time you
parse a type, you can check if the type is in the typedef structure. If it is
found, you can just replace it at the point.

As far as I read the spec, we can assume that typedef should appear before
being used. So you don't need to apply the replacement at the end of parsing.
http://www.w3.org/TR/WebIDL/#idl-typedefs