Bug 117545 – [GTK] Remove legacy hack in CodeGeneratorGObject.pm

• Component: Bindings
• Status: RESOLVED FIXED

[Diego Pino]

There's a piece of code that sets gtype to uint in case it's ushort. gtype is a value obtained from GetGValueTypeName(), which never returns ushort. The snippet is marked with a comment "FIXME: get rid of this glitch?"

[Diego Pino]

Created attachment 204441 [details]
Patch

[Diego Pino]

Created attachment 205012 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 205012 [details]
Patch

Rejecting attachment 205012 [details] from commit-queue.

[e-mail] does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.

[Carlos Garcia Campos]

Comment on attachment 205012 [details]
Patch

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

> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:548
> +    } elsif ($attribute->signature->extendedAttributes->{"ImplementedBy"}) {
> +        my $implementedBy = $attribute->signature->extendedAttributes->{"ImplementedBy"};
> +        $implIncludes{"${implementedBy}.h"} = 1;

I wonder if we can get rid of this here, since this is already done a few lines above in this function.

> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:551
> +        push(@txtGetProps, "        g_value_set_$gtype(value, ${convertFunction}${getterFunctionName}(" . join(", ", @getterArguments) .  ")${postConvertFunction});\n");
> +    } else {
> +        push(@txtGetProps, "        g_value_set_$gtype(value, ${convertFunction}${getterFunctionName}(" . join(", ", @getterArguments) . ")${postConvertFunction});\n");

I wonder why these lines are duplicated, I know it's not introduced by you, but I think this can be simplified. If we can get rid of the implemented by check this could just be 

} else { 
    push(@txtGetProps, "        g_value_set_$gtype(value, ${convertFunction}${getterFunctionName}(" . join(", ", @getterArguments) . ")${postConvertFunction});\n");
}

[Diego Pino]

Comment on attachment 205012 [details]
Patch

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

>> Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:551
>> +        push(@txtGetProps, "        g_value_set_$gtype(value, ${convertFunction}${getterFunctionName}(" . join(", ", @getterArguments) . ")${postConvertFunction});\n");
> 
> I wonder why these lines are duplicated, I know it's not introduced by you, but I think this can be simplified. If we can get rid of the implemented by check this could just be 
> 
> } else { 
>     push(@txtGetProps, "        g_value_set_$gtype(value, ${convertFunction}${getterFunctionName}(" . join(", ", @getterArguments) . ")${postConvertFunction});\n");
> }

Correct. I agree with these suggestions. I will cook a new patch with this changes, and check the script generates the same code.

[Diego Pino]

Created attachment 208011 [details]
Patch

[Build Bot]

Comment on attachment 208011 [details]
Patch

Attachment 208011 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1330178

New failing tests:
svg/batik/filters/feTile.svg

[Build Bot]

Created attachment 208019 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.4

[WebKit Commit Bot]

Comment on attachment 208011 [details]
Patch

Clearing flags on attachment: 208011

Committed r153697: <http://trac.webkit.org/changeset/153697>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.