This package builds with -Werror, and GCC 4.6 triggers new warnings
which will make the package fail to build.  Currently a Debian patch
just passes
    -Wno-error=unused-but-set-variable and
    -Wno-error=unused-but-set-parameter
to avoid build failures, but this patch will be reverted with the
GCC 4.6.1 release, and the severity of the report will be raised.

The full build log can be found at:
http://people.debian.org/~doko/tmp/werror/jigit_1.18-1_lsid64.buildlog
The last lines of the build log are at the end of this report.

reassign 625357 gcc-4.6
thanks

I'll remove the -Werror to stop gcc breaking the build here, but I
definitely believe that gcc is doing the wrong thing here.
Technically, yes - the variables are set but unused. However, this is
a far higher level of pedantry than is warranted for -Wall. The code
being complained about is perfectly valid, typical of the defensive
programming pattern of "initialise all variables". I hence reject this
as a bug.

Hi Steve,

Steve McIntyre wrote:

The -Wall option has implied -Wunused-variable for a very long time;
the idea as I understand it was to detect dead code which might be an
indicator for other problems (typos or planned functionality that was
left out) and which is distracting.  As I understand it, the only
reason -Wunused-but-set-variable did not exist until recently was a
small accident of implementation, that technically "x = 5;" is an
expression that evaluates "x".

I believe the best way to avoid breaking the build is to simply not use
-Werror on autobuilders.  I would like to teach autobuilders to send
warnings by email to interested maintainers so maintainers could get the
benefit of notification without making the release team and toolchain
maintainers' lives more difficult; would you be interested in that?

Thanks and hope that helps,
Jonathan

severity 625357 normal
retitle 625357 gcc -Wunused-but-set-variable should not be implied by -Wall (?)
tags 625357 = upstream moreinfo
quit

Hi again,

Steve McIntyre wrote:

 $ program_one='int main(void) { int x = 5; return 0; }'
 $ program_two='int main(void) { int x; x = 5; return 0; }'
 $ echo "$program_one" | gcc-4.4 -Wall -x c -
 <stdin>: In function ‘main’:
 <stdin>:1: warning: unused variable ‘x’
 $ echo "$program_two" | gcc-4.4 -Wall -x c -

So imho gcc before v4.6 is just fundamentally confused.  Which means
there are a number of ways one could go with this.  Do you mean:

 A. Unused variables are not a big deal, and they belong in -Wextra,
    not -Wall.

 B. New warnings are a pain in the neck and should go in -Wextra
    during a transition period.

 C. The idiom

	ssize_t unused;
	/*
	 * Yes, there might be an error, dear gcc -D_FORTIFY_SOURCE,
	 * but we want to ignore it.
	 */
	unused = write(...);

   in place of, say,

	/* loop on partial writes and EINTR */
	xwrite(...);

   is good style and the -Wunused-but-set-variable warning is
   fundamentally misguided.

 D. The idiom

	int x;
	int y;

	... ordinary code ...
	#if SOMETIMES
	... code to set and use x ...
	#endif

    in place of

	#if SOMETIMES
	int x;
	#endif
	int y;

	... ordinary code ...
	#if SOMETIMES
	... code to set and use x ...
	#endif

    or

	int y;

	... ordinary code ...
	#if SOMETIMES
	{
		int x;
		... code to set and use x ...
	}
	#endif

    is good style and the longstanding -Wunused-variable warning is
    fundamentally misguided.

 D. Something else?

Sorry for the lack of clarity before.
Jonathan

The -Werror flag itself isn't the real problem for me, and I've
already removed it in my upload. I'm much more worried about (IMHO)
broken decisions being made in gcc about the levels of some
warnings. See my second mail in a few minutes...

Potentially, yes.

Sure, that's confused. :-)

No.

Maybe, but a lot of people will never see them anyway until they cause
build failures. :-)
various things to respond to you here. It looks like I've missed
*exactly* the thing gcc was complaining about, and it is case C
here. Now I can see that, I'm less unhappy about the warning.

My code is analogous to your example code:

static int parse_md5_entry(char *md5_entry)
{
    int error = 0;
    char *file_name = NULL;
    char *md5 = NULL;
    unsigned char bin_md5[16];
    int i;

    ...
    /* code that does the real meat of the work to fill in "md5" and
       "filename" , but doesn't set/use "error" at all */
    ...

    error = add_md5_entry(UNKNOWN, md5, file_name);
    return 0;
}

To explain a bit more: the warning reported by gcc (unhelpfully)
points at the declaration/initialisation of "error", which led me to
(incorrectly) think that it was complaining about set-but-unused for
the initialisation. With minor tweaks, I can see that it's complaining
about the return value from add_md5_entry() being ignored but doesn't
*say* that. :-(

I'm about to update the code around that area now. More accurate
diagnostics from gcc would be a major improvement, I think!

tags 625357 - moreinfo
retitle 62537 gcc -Wunused-but-set-variable warning is confusing
# bad error message
severity 625357 minor
quit

Steve McIntyre wrote:

Ah, I see and I think I agree.  The current message

 test.c:3:6: warning: variable ‘x’ set but not used [-Wunused-but-set-variable]

places the emphasis on the assignment (which is not worth warning
about), when the warning is instead supposed to be about the variable
(which is never used).  Something like

 test.c:3:6: warning: unused variable ‘x’ [-Wunused-but-set-variable]

would be much clearer.

Will think more and then work on a patch.  Thanks for your patient
explanations.

reassign 625357 src:gcc-snapshot 20111210-1
affects 625357 + gcc-4.6
affects 625357 + gcc-4.7
quit

Jonathan Nieder wrote:
[...]

Here's a rough patch.  Will send upstream after testing, but thoughts
welcome already.  Sensible?

Jonathan, what's the upstream status for this issue?