Bug 157336 – Start on dictionary support for IDL, getting enough to work for one dictionary

• Status: RESOLVED
• Resolution: FIXED
• Component: Bindings

[Darin Adler]

Start on dictionary support for IDL, getting enough to work for one dictionary

[Darin Adler]

Created attachment 278067 [details]
Patch

[Alex Christensen]

Comment on attachment 278067 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=278067&action=review

Looks mostly good, but I didn't look at all the details yet.

> Source/WebCore/bindings/scripts/CodeGenerator.pm:401
> +    return $dictionaryTypes{$type} || 0;

|| 0?

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:4094
> -    my $function = shift;
> -    my $functionString = shift;
> -    my $indent = shift;
> -    my $svgPropertyType = shift;
> -    my $interfaceName = shift;
> +    my ($function, $functionString, $indent, $svgPropertyType, $interface) = @_;

Using shift for each parameter gives us a history of when each parameter was added.  Is there an advantage to putting everything on one line, other than it looks good?  Is there a WebKit perl style consensus on this?

> Source/WebCore/bindings/scripts/test/ObjC/DOMTestObj.mm:1573
> -- (DOMbool *)strictFunction:(NSString *)str a:(float)a b:(int)b
> +- (BOOL)strictFunction:(NSString *)str a:(float)a b:(int)b

There are lots of changes from DOMbool to BOOL or gboolean.  Does this change the API at all?

[Darin Adler]

Comment on attachment 278067 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=278067&action=review

> Source/WebCore/ChangeLog:98
> +        the type "boolean", which doesn't exist in IDL.

Oops, this should have said the type "bool", which doesn’t exist in IDL.

>> Source/WebCore/bindings/scripts/CodeGenerator.pm:401
>> +    return $dictionaryTypes{$type} || 0;
> 
> || 0?

Yes, that’s a perl idiom that turns things like "undefined" into 0.

>> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:4094
>> +    my ($function, $functionString, $indent, $svgPropertyType, $interface) = @_;
> 
> Using shift for each parameter gives us a history of when each parameter was added.  Is there an advantage to putting everything on one line, other than it looks good?  Is there a WebKit perl style consensus on this?

This style is better.

I don’t agreel that separate change history for each "shift" line is valuable. If something thought that was valuable, then that person might ask us to format C++ code like this:

    int f(
          const char* a,
          long b,
          RefPtr<X> c
    ) {

I say, no.

>> Source/WebCore/bindings/scripts/test/ObjC/DOMTestObj.mm:1573
>> +- (BOOL)strictFunction:(NSString *)str a:(float)a b:(int)b
> 
> There are lots of changes from DOMbool to BOOL or gboolean.  Does this change the API at all?

No, these changes are only because I fixed broken test cases that were trying to use "bool" instead of "boolean". The generated code in that case would not have compiled. So no, it won’t be changing behavior of actual WebKit code. We could only get away with this mistake in the tests where the code output is not compiled.

[Darin Adler]

Committed r200448: <http://trac.webkit.org/changeset/200448>