xbolva00 marked 2 inline comments as done.
xbolva00 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>;
----------------
jfb wrote:
> xbolva00 wrote:
> > jfb wrote:
> > > 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}"
> > But it is very specific.. cannot be used for example here:
> >
> > bool t() { return 0 * x; }
> >
> > In the place where the check is performed we know nothing whether expr is
> > assigned/returned/etc.
> >
> > I don’t believe we have to produce message with such details.
> All I'm saying is that "boolean context" ins't something people will
> understand.
> Maybe "converting the result of a multiplication to a boolean always yields
> {true|false}"?
+1, like it
================
Comment at: test/SemaCXX/warn-int-in-bool-context.cpp:96
+
+ if (f == apple || orange) // expected-warning {{enum constant in boolean
context}}
+ return a;
----------------
jfb wrote:
> xbolva00 wrote:
> > jfb wrote:
> > > xbolva00 wrote:
> > > > xbolva00 wrote:
> > > > > @aaron.ballman : In C (but not in C++ ugh?) mode we have: 'use of
> > > > > logical '||' with constant operand' here..
> > > > >
> > > > > /home/xbolva00/LLVM/llvm/tools/clang/test/SemaCXX/warn-int-in-bool-context.cpp:106:18:
> > > > > warning: use of logical '||' with constant operand
> > > > > if (f == apple || orange) // expected-warning {{enum constant in
> > > > > boolean context}}
> > > > > ^ ~~~~~~
> > > > > /home/xbolva00/LLVM/llvm/tools/clang/test/SemaCXX/warn-int-in-bool-context.cpp:106:18:
> > > > > note: use '|' for a bitwise operation
> > > > > if (f == apple || orange) // expected-warning {{enum constant in
> > > > > boolean context}}
> > > > > ^~
> > > > > |
> > > > > /home/xbolva00/LLVM/llvm/tools/clang/test/SemaCXX/warn-int-in-bool-context.cpp:106:21:
> > > > > warning: enum constant in boolean context
> > > > > if (f == apple || orange) // expected-warning {{enum constant in
> > > > > boolean context}}
> > > > >
> > > > >
> > > > >
> > > > >
> > > > What is your opinion here (^^) , @jfb ?
> > > >
> > > > Thanks.
> > > C, C++, ObjC, and ObjC++ should emit different diagnostics here. That
> > > being said, none of these diagnostics are particular easy to understand.
> > > I don't think we should ever say "boolean context" anywhere.
> > So I will make this as it was - C++ only.
> It'll work in the other languages though, right?
Sight, making it C++ only leaves C with not diagnosed case:
_Bool f() { return orange; }
And I don’t know why ‘use of logical || with constant operand’ is C only :/..
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
