These warnings are prevalent when building with CMake.
Created attachment 226572 [details] Patch
Comment on attachment 226572 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=226572&action=review OK. > Source/WebCore/bindings/gobject/WebKitDOMObject.cpp:28 > + switch (propertyId) { > default: > - G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); > + G_OBJECT_WARN_INVALID_PROPERTY_ID(object, propertyId, pspec); > break; > } The switch statement is unnecessary as we always execute the code for the default case. > Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:661 > + push(@txtGetProps, "#else\n") if $conditionalString; > + push(@txtGetProps, " UNUSED_PARAM(value);\n") if $conditionalString; > + push(@txtGetProps, "${conditionGuardEnd}\n") if $conditionalString; Unless you feel the duplication of the if conditional ("if $conditionalString") improves the readability of this code, I would write this as: if ($conditionalString) { push(@txtGetProps, "#else\n"); push(@txtGetProps, " UNUSED_PARAM(value);\n"); push(@txtGetProps, "${conditionGuardEnd}\n"); } > Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:674 > + push(@txtSetProps, "#else\n") if $conditionalString; > + push(@txtSetProps, " UNUSED_PARAM(value);\n") if $conditionalString; > + push(@txtSetProps, "${conditionGuardEnd}\n") if $conditionalString; Similarly, I would write this as: if ($conditionalString) { push(@txtSetProps, "#else\n"); push(@txtSetProps, " UNUSED_PARAM(value);\n"); push(@txtSetProps, "${conditionGuardEnd}\n"); } > Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:797 > + push(@cBodyProperties, "UNUSED_PARAM(request);\n"); Nit: We should indent "UNUSED_PARAM(request);". > Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestNode.cpp:101 > +UNUSED_PARAM(request); Nit: This line should be indented 4 spaces.
Comment on attachment 226572 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=226572&action=review Thank you! > Source/WebCore/bindings/gobject/DOMObjectCache.cpp:59 > -static void weakRefNotify(gpointer data, GObject* zombie) > +static void weakRefNotify(gpointer data, GObject* /* zombie */) I think e can simply avoid the parameter name in this case. >> Source/WebCore/bindings/gobject/WebKitDOMObject.cpp:28 >> } > > The switch statement is unnecessary as we always execute the code for the default case. The whole function is unnecessary :-) We can simply remove it and not give any imp for GObjectClass::get_property, since the only property this object has is not readable.
Comment on attachment 226572 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=226572&action=review >>> Source/WebCore/bindings/gobject/WebKitDOMObject.cpp:28 >>> } >> >> The switch statement is unnecessary as we always execute the code for the default case. > > The whole function is unnecessary :-) We can simply remove it and not give any imp for GObjectClass::get_property, since the only property this object has is not readable. I hope you don't mind if I don't take the suggestion to remove the method entirely. I don't have time to rebuild and test right now, but I want to land this get it off my plate. The DOM bindings and code generator need *a lot* of cleanup, so we should definitely think about running through them again pre-3.0.
(In reply to comment #4) > (From update of attachment 226572 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=226572&action=review > > >>> Source/WebCore/bindings/gobject/WebKitDOMObject.cpp:28 > >>> } > >> > >> The switch statement is unnecessary as we always execute the code for the default case. > > > > The whole function is unnecessary :-) We can simply remove it and not give any imp for GObjectClass::get_property, since the only property this object has is not readable. > > I hope you don't mind if I don't take the suggestion to remove the method entirely. I don't have time to rebuild and test right now, but I want to land this get it off my plate. The DOM bindings and code generator need *a lot* of cleanup, so we should definitely think about running through them again pre-3.0. Sure, go ahead and fix the warnings :-)
Committed r165618: <http://trac.webkit.org/changeset/165618>