[webkit-reviews] review denied: [Bug 82599] IDLParser.pm should be able to parse sequence<T> as method argument : [Attachment 134636] updated_patch

[bugzilla-daemon at webkit.org]

Kentaro Hara <haraken at chromium.org> has denied Vineet Chaudhary (vineetc)
<rgf748 at motorola.com>'s request for review:
Bug 82599: IDLParser.pm should be able to parse sequence<T> as method argument
https://bugs.webkit.org/show_bug.cgi?id=82599

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

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


> Source/WebCore/bindings/scripts/CodeGeneratorCPP.pm:472
> +		   if ($codeGenerator->GetArrayType($param->type)) {
> +		       next TOP;
> +		   }

Scattering this kind of code around is a bit dirty. Shall we put the code in
ShouldSkipType($function)? e.g.

sub ShouldSkipType() {
  ...;
  foreach my $param (@{$function->parameters}) {
    return 1 if $codeGenerator->GetArrayType($param->type);
  }
  ...;
}

Same comments for all "next TOP" in this patch.

> Source/WebCore/bindings/scripts/CodeGeneratorObjC.pm:848
>	   foreach my $function (@{$dataNode->functions}) {

You can put something like ShouldSkipType() here. Maybe you need to prepare
ShouldSkipType() in CodeGeneratorObjC.pm.

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:3606
> +		       AddToImplIncludes($arrayType);

This should be "${arrayType}.h"