[cfe-commits] [PATCH] Properly check for unused comparisons in arguments to function-like macros

[Richard Smith]

On Thu, Jan 10, 2013 at 2:39 PM, Matt Beaumont-Gay <matthewbg at google.com> wrote:
>
>   I sort of don't care about the macro body case right now, both since I don't want to enshrine the current (sort of dumb) behavior in a test case, and since I'm going to fix it shortly with test cases for the correct behavior.
>
>   Getting clever with looking at the sources of the start and end locations sounds like... more code. I recall you saying something recently about the YAGNI philosophy? :)
>
>   Richard, I don't quite understand your test case, and besides I think it's a test for -Wunused-value rather than -Wunused-comparison?

Yes, you're right about it being for -Wunused-value, not
-Wunused-comparison. There are two relevant things happening there:

1) The operator (let's imagine it was a comparison operator) comes
from a macro argument which is used twice, once inside the guts of the
macro, and once to produce a result which is discarded. IIRC, the
"macro does some other stuff then produces a value which the caller
may not care about" pattern is exactly what we're trying to suppress
here. Here's a simpler example of that:

SET_VALUE_IF(cond, val) cond ? value = val : val
int k;
#define SET_VALUE_IF(x) ((k x) && (value = 1), k x)
SET_VALUE_IF(== 5); // Warn on unused comparison here?

The comparison in the macro argument isn't actually unused, because
the argument is expanded twice.

2) The operator actually comes from a macro body, but your test won't
notice that because it's in a macro arg expansion too.

Anyway, let's get the simple thing out before worrying about these; patch LGTM.