[webkit-reviews] review denied: [Bug 87185] [V8] Refactor generation code for non-standard functions : [Attachment 143403] Patch

[bugzilla-daemon at webkit.org]

Kentaro Hara <haraken at chromium.org> has denied Kent Tamura
<tkent at chromium.org>'s request for review:
Bug 87185: [V8] Refactor generation code for non-standard functions
https://bugs.webkit.org/show_bug.cgi?id=87185

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

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


IsStandardFunction() looks good, but I am neutral to the refactoring of
GenerateNonStandrdFunction(). It seems that GenerateNonStandrdFunction() "just"
factors out a code, which would not be so beneficial.

> Source/WebCore/ChangeLog:11
> +	   (IsStandardFunction): Introduce a new functio to check if a

Nit: function

> Source/WebCore/ChangeLog:15
> +	   (GenerateImplementation): Use IsStandardFunction and
GenerateNonStandrdFunction.

Nit: GenerateNonStandardFunction.

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:2127
> +sub GenerateNonStandrdFunction

GenerateStandardFunction

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:2206
> +	   die "This shouldn't happen: " . IsStandardFunction($function) .
"\n";

How does this message help? IsStandardFunction($function) would just output 0
or 1?

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:-2721
> -	       $num_callbacks++;

This line seems to be lost in your patch.

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:2762
> +	   next if IsStandardFunction($dataNode, $function);

Why is this check needed? It seems to be already checked above.

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:2763
> +	   GenerateNonStandrdFunction($dataNode, $function);

GenerateNonStandardFunction