GNOME Bugzilla – Bug 357510
glib-genmarshal: generate -Wunused-clean code
Last modified: 2007-06-19 12:10:21 UTC
Compilation of glib-genmarshal generated code with -Wunused results in warnings like: gwydgetmarshals.c: In function '_gwydget_marshal_VOID__INT_PARAM': gwydgetmarshals.c:54: warning: unused parameter 'return_value' gwydgetmarshals.c:57: warning: unused parameter 'invocation_hint' They can be removed by emitting G_GNUC_UNUSED to the C code.
Created attachment 73339 [details] [review] proposed patch The attached patch adds G_GNUC_UNUSED to invocation_hint argument in the body code unconditionally, and to return_value when the signal has no return value. (Never mind patch name, it applies to 2.12 too.)
Created attachment 73737 [details] [review] alternative patch Does this look ok ?
(In reply to comment #2) > Created an attachment (id=73737) [edit] > alternative patch > > Does this look ok ? Either bugzilla is making fun of me, or atachment #73737 is a gspawn fdwalk() patch totally unrelated to this bug. Unfortunately, bugzilla doesn't let me obsolete your patch so I'm not reattaching it, but the first patch still stands.
sorry, wrong bug
*** Bug 359165 has been marked as a duplicate of this bug. ***
Confirming this bug. The duplicate at bug 359165 contains both a test case and a similar patch.
It does not appear that glib-genmarshal has been patched to address this bug yet. Is there something holding this bug up (other than the availability of round tuits)?
This patch is important for swfdec, because a) I want to enabled unused parameter warnings and b) it annoys package (in particular RPM) builders that build straight out of git. Here's the boring details: swfdec adds the warning flags "-Wall -Wextra -Wno-unused-parameter -Werror" when building out of git. The RPM build adds "-Wall" again. This causes the warning flags on the gcc command line to be "-Wall -Wextra -Wno-unused-parameter -Werror -Wall". Warning flags are parsed by gcc left to right. Since "-Wall" enables unused parameter warnings if "-Wextra" is defined, the RPM build suddenly has unused paramter warnings enabled and breaks in files generated with glib-genmarshal.
the above patch/comment history is confusing. what's the exact patch candidate for inclusion now?
(In reply to comment #9) > the above patch/comment history is confusing. what's the exact patch candidate > for inclusion now? The one in comment #1. Also attachment #73909 [details]. The former only marks return_value as G_GNUC_UNUSED if in a setter. I'm not sure of the intended logic here.
hm, bug 359165 has a nice clean patch and test case. especially with the mess above, this bug should have been closed as dup instead. *** This bug has been marked as a duplicate of 359165 ***
(In reply to comment #10) > The one in comment #1. Also attachment #73909 [details] [edit]. The former only marks > return_value as G_GNUC_UNUSED if in a setter. I'm not sure of the intended > logic here. Yes, it is the intended logic. (In reply to comment #11) > hm, bug 359165 has a nice clean patch and test case. I maintain the point marking only actually unused vars is better -- cleaner, whatever. Otherwise we could just mark all generated code G_GNUC_UNUSED... This is not what the ,cleaner` patch in bug 359165 does. > especially with the mess above, I'm sorry about that, but I didn't cause it.
(In reply to comment #12) > (In reply to comment #11) > > hm, bug 359165 has a nice clean patch and test case. > > I maintain the point marking only actually unused vars is better -- cleaner, > whatever. Otherwise we could just mark all generated code G_GNUC_UNUSED... > > This is not what the ,cleaner` patch in bug 359165 does. i'm not sure i fully understand your argument here, so please correct me if i'm wrong. it seems to me you're arguing that your change: - g_fprintf (fout, "%sGValue *return_value,\n", indent (ind)); + if (sig->rarg->setter) + g_fprintf (fout, "%sGValue *return_value,\n", + indent (ind)); + else + g_fprintf (fout, "%sG_GNUC_UNUSED GValue *return_value,\n", + indent (ind)); is better then the change from #359165: - g_fprintf (fout, "%sGValue *return_value,\n", indent (ind)); + g_fprintf (fout, "%sGValue *return_value G_GNUC_UNUSED,\n", + indent (ind)); if that's the case, i can just say i don't second your opinion, the latter form is obviously leaner and also covers all cases where gcc can potentially throw an unused warning. i also don't second your opinion that from applying the patch from #359165 would follow that we had to apply additional G_GNUC_UNUSED in generated code where it's useless.