[Webkit-unassigned] [Bug 108799] [CPP, GObj, ObjC] Add checks for unsupported Web IDL features, remove #if V8/JS

[bugzilla-daemon at webkit.org]

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





--- Comment #6 from Nils Barth <nbarth at google.com>  2013-02-04 04:02:18 PST ---
To summarize:
* Most skip checks are existing checks, just mixed with new changes in diff.
* Check for array return on function in GObject *is* new, and took some looking.
Q: Proper order is Sequence then Array -- should I fix this in CPP? (minor)

(In reply to comment #3)
> (From update of attachment 186336 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=186336&action=review
> 
> > Source/WebCore/bindings/scripts/CodeGeneratorCPP.pm:189
> > +    if ($codeGenerator->GetArrayType($type)) {
> > +        return 1;
> > +    }
> > +
> > +    if ($codeGenerator->GetSequenceType($type)) {
> > +        return 1;
> > +    }
> 
> Are all array types and sequence types not supported in CodeGeneratorCPP? (not only DOMStringList and DOMString[])

Yes, array an sequence are not supported; these are not new tests
(it just looks weird b/c I'm making a few changes).

I added
my $type = $function->signature->type
when testing for DOMStringList -- the new code is more legible,
but b/c I'm adding 2 other tests immediately below, the diff mixes the two.

Before:
    if ($codeGenerator->GetArrayType($function->signature->type)) {
        return 1;
    }

    if ($codeGenerator->GetSequenceType($function->signature->type)) {
        return 1;
    }


After:
    if ($codeGenerator->GetArrayType($type)) {
        return 1;
    }

    if ($codeGenerator->GetSequenceType($type)) {
        return 1;
    }

    if ($type eq "DOMStringList") {
        return 1;
    }

    if ($function->{overloads} && @{$function->{overloads}} > 1) {
        return 1;
    }

> > Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:260
> > +    if ($codeGenerator->GetArrayType($functionReturnType)) {
> > +        return 1;
> > +    }
> 
> Ditto.

This check for array return on function in GObject *is* new;

This took a bit of looking at, and I noted it in the ChangeLog
(it was the last change I noticed and made):
5. [GObj] Skip array return type (to handle DOMString[])

GObject already does not allow array type on attributes, or sequence types anywhere -- the only time it checks for GetArrayType is to skip these, never to generate code, so I'm pretty sure this is just an omission, which I'm fixing.

> Also, what about $codeGenerator->GetSequenceType() ?

$codeGenerator->GetSequenceType() is above this code, hence not in patch.

Following Web IDL, one should have Sequence before Array, as per:
3.10.22. Sequences — sequence<T>
http://www.w3.org/TR/WebIDL/#idl-sequence
3.10.23. Arrays — T[]
http://www.w3.org/TR/WebIDL/#idl-array

GObject and ObjC correctly have sequence before array, but CPP flips them.
I didn't fix this order in CodeGeneratorCPP.pm (seemed a bit minor);
should I?

> > Source/WebCore/bindings/scripts/CodeGeneratorObjC.pm:438
> > +    return 1 if $codeGenerator->GetSequenceType($type);
> > +    return 1 if $codeGenerator->GetArrayType($type);
> 
> Ditto.

As above, not a new change -- just the $type = $function->signature->type getting mixed with the two new tests.

Before:
    return 1 if $codeGenerator->GetSequenceType($function->signature->type);
    return 1 if $codeGenerator->GetArrayType($function->signature->type);

After:
    return 1 if $codeGenerator->GetSequenceType($type);
    return 1 if $codeGenerator->GetArrayType($type);
    return 1 if $type eq "DOMStringList";

    if ($function->{overloads} && @{$function->{overloads}} > 1) {
        return 1;
    }

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