On Thu, 2016-08-25 at 11:35 -0600, Martin Sebor wrote:
> > +foo (int a, int b)
> > +{
> > + int r = 0;
> > + r += !a == (a < b);
> > + r += !a == (a > b);
> > + r += !a == (a >= b);
> > + r += !a == (a <= b);
> > + r += !a == (a != b);
> > + r += !a == (a == b);
> > + r += !a == (a || b);
> > + r += !a == (a && b);
> > + r += !a == (!b);
> > +
> > + r += !a == (a ^ b); /* { dg-warning "logical not is only applied
> > to the left hand side of comparison" } */
> > + r += !a == (a | b); /* { dg-warning "logical not is only applied
> > to the left hand side of comparison" } */
> > + r += !a == (a & b); /* { dg-warning "logical not is only applied
> > to the left hand side of comparison" } */
>
> A question more than a comment: warning on the last three expressions
> above makes sense to me when the operands are integers but I'm less
> sure that the same logic applies when the operands are Boolean. Does
> it make sense to issue the warning for those given that (a | b) and
> (a & b) are the same as (a || b) and (a && b) for which the warning
> is suppressed?
>
> In other words, is warning on the latter of the two below but not on
> the former a good idea or should they be treated the same way?
>
> $ cat t.c && gcc -S -Wlogical-not-parentheses t.c
> _Bool f (int a, _Bool b, _Bool c)
> {
> _Bool r;
>
> r = !a == (b || c);
> r = !a == (b | c);
>
> return r;
> }
> t.c: In function âfâ:
> t.c:6:10: warning: logical not is only applied to the left hand side
> of
> comparison [-Wlogical-not-parentheses]
> r = !a == (b | c);
> ^~
> t.c:6:7: note: add parentheses around left hand side expression to
> silence this warning
> r = !a == (b | c);
> ^~
> ( )
>
> Also, having hardly any experience with the fixit hints, I'm not
> sure how helpful this one is. It seems really hard to tell what
> it suggests as the fix (I had to try it both ways to see). I
> think would be a lot clearer if it showed the full expression
> rather than just the operators. Would printing this instead be
> doable?
>
> r = !a == (b | c);
> ^~
> (!a)
This seems to me to be more about how we print fix-it hints in general
that about the specific usage of them by this diagnostic.
It would be possible to rewrite the fix-it printing routine in
diagnostic-show-locus.c to print the affected parts of the line
(perhaps using the edit_context class from my recent patch kit).
