On Jun 1, 2011, at 5:52 AM, Hans Wennborg wrote:
> This is an attempt to address some of
> http://llvm.org/bugs/show_bug.cgi?id=9969
>
> The idea is to warn about code such as:
>
> int foo(int x, bool someCondition) {
> return x + someCondition ? 42 : 0;
> }
>
> where it seems likely that the programmer forgot that ?: has lower
> precedence than +.
>
> The patch looks for condition expressions which are non-boolean binary
> operators, implicitly cast to bool, where the right-hand side
> expression is bool.
Cool. A few comments:
+/// DiagnoseConditionalPrecedence - Emit a warning when a conditional operator
+/// and binary operator are mixed in a way that suggests the programmer assumed
+/// the conditional operator has higher precedence, for example:
+/// "int x = a + someBinaryCondition ? 1 : 2".
+static void DiagnoseConditionalPrecedence(Sema &Self,
+ SourceLocation OpLoc,
+ Expr *cond,
+ Expr *lhs,
+ Expr *rhs) {
+ if (ImplicitCastExpr *CE = dyn_cast<ImplicitCastExpr>(cond)) {
+ if (BinaryOperator* OP = dyn_cast<BinaryOperator>(CE->getSubExpr())) {
+ if (OP->getType()->isBooleanType())
+ return;
This isn't going to work in C, because comparison operators in C have type
'int'. Rather than looking at the type of the BinaryOperator, why not look for
specific operators (+, -, etc.) that are likely to be misunderstood?
Also, it looks like your patch is missing the required changes to
DiagnosticSemaKinds.td.
Finally, did you try running this patch over (say) the LLVM + Clang code base,
to evaluate its signal/noise ratio?
- Doug
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
