jfb added inline comments.
================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5609
+def warn_mul_in_bool_context : Warning<
+ "'*' in bool context, maybe you mean '&&'?">,
+ InGroup<IntInBoolContext>;
----------------
aaron.ballman wrote:
> xbolva00 wrote:
> > aaron.ballman wrote:
> > > I would appreciate seeing some motivating examples for this case, because
> > > it seems like the `&&` suggestion is misleading more often than it's
> > > helpful in the current test cases.
> > >
> > > Also, what is a "bool context"?
> > Bool context - places where we expect boolean expressions.
> >
> > If you have idea for better terminology, I am happy to apply it.
> >
> > Yeah, I will remove “&&” suggestion..
> I don't have a good recommendation for a replacement for "boolean context",
> so I suppose we can leave it.
IMO it's clearer to say "assigning the result of a multiplication to a boolean
variable always yields {true|false}"
================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5612
+def warn_left_shift_in_bool_context : Warning<
+ "'<<' in boolean context, maybe you mean '<'?">,
+ InGroup<IntInBoolContext>;
----------------
xbolva00 wrote:
> xbolva00 wrote:
> > I will this line.
> * Ehh.. I will fix this line to use "did you mean".
Same here, "assigning the result of a left shift to a boolean variable always
yields {true|false}"
Same for all the other ones...
================
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
'<'?}}
+
----------------
I'm not sure the "did you mean" part is helpful. Do we have data showing that
this is what people actually mean?
================
Comment at: test/Sema/warn-int-in-bool-context.c:32
+ r = a ? -2 : 0;
+ r = a ? 3 : -2;
+ r = a ? 0 : TWO; // expected-warning {{'?:' with integer constants in
boolean context}}
----------------
Why wouldn't think one warn?
================
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'}}
----------------
Why does this one warn? It doesn't always yield the same result.
================
Comment at: test/Sema/warn-unreachable.c:147
+ // expected-warning@+1 {{'*' in boolean context, the expression will always
evaluate to 'false'}}
if (0 * x) calledFun(); // expected-warning {{will never be executed}}
}
----------------
It seems like here the "will never be executed" warning is more useful. Do we
want to emit both?
================
Comment at: test/SemaCXX/warn-int-in-bool-context.cpp:99
+
+ if (ans == yes || no) // expected-warning {{enum constant in boolean
context}}
+ return a;
----------------
aaron.ballman wrote:
> xbolva00 wrote:
> > aaron.ballman wrote:
> > > Why do we want to diagnose this case in C++ but not in C?
> > I think we can and should but for unknown reason (for me) this is GCC’s
> > behaviour.
> >
> > Maybe it is too noisy for C codebases?
> >
> > Maybe introduce -Wenum-in-bool-context as subgroup of
> > -Wint-in-bool-context? And enable it for C and C++?
> I think it's just a bug in GCC (they use separate frontends for C and C++, so
> these sort of differences crop up sometimes). I think this should be safe to
> diagnose the same in C and C++.
Agreed.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63082/new/
https://reviews.llvm.org/D63082
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
