[Webkit-unassigned] [Bug 26398] Move IDL extended attributes to the location specified in WebIDL

[bugzilla-daemon at webkit.org]

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





--- Comment #26 from yosin at chromium.org  2012-07-13 07:00:53 PST ---
(From update of attachment 151870)
View in context: https://bugs.webkit.org/attachment.cgi?id=151870&action=review

> Source/WebCore/bindings/scripts/IDLParser.pm:314
> +    for my $item (@{$interfaceMembers}) {

How about this?

$dataNode->attributes = [ grep { ref($_) eq 'domAttribute' } @$interfaceMembers ];
$dataNode->constants = [ grep { ref($_) eq 'domConstant' } @$interfaceMembers ];
$dataNode->functions = [ grep { ref($_) eq 'domFunction' } @$interfaceMembers ];

I know this is slower than for-loop. But, we may know what attributes, constants and function have at glance.

> Source/WebCore/bindings/scripts/IDLParser.pm:512
> +    for my $item (@{$exceptionMembers}) {

This is fragment as same as in parseInterfaceNew.
Is it better to have subroutine for sharing code?

> Source/WebCore/bindings/scripts/IDLParser.pm:957
> +    if ($backwardCompatibleMode) {

nit: return $backwardCompatibleMode ? $self->parseArgumentOld() : $self->parseAgumentNew();

or

if ($backwardCompatibleMode) {
    return $self->parseArgumentOld();
}
return $self->parseArgumentNew();

> Source/WebCore/bindings/scripts/IDLParser.pm:1008
> +    return;

nit: Do we need to have "return" at end of subroutine?

> Source/WebCore/bindings/scripts/IDLParser.pm:1018
> +    return;

nit: Do we need to have "return" at end of subroutine?

> Source/WebCore/bindings/scripts/IDLParser.pm:1114
> +    my %attr = ();

nit: return {};

> Source/WebCore/bindings/scripts/IDLParser.pm:1134
> +        return \%attr;

How about writing below?

my $attrs = {};
...
$attrs->{$name} = ...;

return $attrs;

This may be easier to put "\" onto each return statement.

> Source/WebCore/bindings/scripts/IDLParser.pm:1216
> +        push(@types,$self->parsePrimitiveOrStringType());

nit: We don't need to have @types to reduce ARRAY copy and to avoid reusing variable.

return join('', $self->parsePimitiveOrStringType(), @{$self->parseTypeSuffix());

This comment applies other usage of @types and join('', @types) in this subroutine.

> Source/WebCore/bindings/scripts/IDLParser.pm:1349
> +        return \@suffix;

nit: return [ "[]", @{$self->parseTypeSuffix() ];

> Source/WebCore/bindings/scripts/IDLParser.pm:1355
> +        return \@suffix;

nit: return [ "?", @{$self->parseTypeSuffixStartingWithArray()} ];

> Source/WebCore/bindings/scripts/IDLParser.pm:1408
> +        return \@names;

nit: return [ $self->parseScopedName(), @{$self->parseScopedNames()} ];

> Source/WebCore/bindings/scripts/IDLParser.pm:1578
> +    for my $item (@{$interfaceMembers}) {

See a comment for parseInterfaceNew and parseExceptionNew.
We may want to share codes.

> Source/WebCore/bindings/scripts/IDLParser.pm:1707
> +    for my $item (@{$exceptionMembers}) {

See a comment for parseInterfaceNew, parseExceptionNew and parseInterfaceOld.
We may want to have "sub" and share code.

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