[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context
JF Bastien via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 30 12:47:39 PDT 2019
jfb added inline comments.
================
Comment at: test/Sema/warn-int-in-bool-context.c:26
+ r = a << 7; // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+ r = ONE << b; // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+
----------------
xbolva00 wrote:
> jfb wrote:
> > I'm not sure the "did you mean" part is helpful. Do we have data showing that this is what people actually mean?
> No, no data, just what GCC suggests.
I would drop it without knowing that it's actually what people usually mean.
================
Comment at: test/Sema/warn-int-in-bool-context.c:33
+ r = a ? 3 : -2;
+ r = a ? 0 : TWO; // expected-warning {{'?:' with integer constants in boolean context}}
+ r = a ? 3 : ONE; // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
----------------
xbolva00 wrote:
> jfb wrote:
> > Why does this one warn? It doesn't always yield the same result.
> GCC warns here..
Our goal isn't to achieve GCC warning parity. I like warnings that fire when it's extremely likely a bug. Here the diagnostic seems to be inconsistent with other instances of this diagnostic. Maybe this should warn, but then other cases should as well (such as `a?3:-2` above). Or maybe it shouldn't warn at all.
================
Comment at: test/Sema/warn-unreachable.c:147
+ // expected-warning at +1 {{'*' in boolean context, the expression will always evaluate to 'false'}}
if (0 * x) calledFun(); // expected-warning {{will never be executed}}
}
----------------
xbolva00 wrote:
> jfb wrote:
> > It seems like here the "will never be executed" warning is more useful. Do we want to emit both?
> Useful but -Wunreachable-code is disabled by default (not part of -Wall).
I don't think we want two diagnostics when one will do.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63082/new/
https://reviews.llvm.org/D63082
More information about the cfe-commits
mailing list