Hi,
This is a follow-up to r132565, and should address the rest of PR9969:
Warn about cases such as
int foo(int a, bool b, int c, int d) {
return a + b ? c : d; // user probably meant a + (b ? c : d);
}
also when + is an overloaded operator call.
Thanks,
Hans
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 1914a6a..f8cf13b 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -6230,10 +6230,70 @@ static bool IsArithmeticOp(BinaryOperatorKind Opc) {
return Opc >= BO_Mul && Opc <= BO_Shr;
}
+/// Returns true if E is an arithmetic binary expression, either using a
+/// built-in or overloaded operator, and sets *OpCode to the opcode and
+/// *RHS to the right-hand side expression.
+static bool IsArithmeticBinaryExpr(Expr *E, BinaryOperatorKind *Opcode,
+ Expr **RHS) {
+ // Remove implicit casts.
+ while (ImplicitCastExpr *C = dyn_cast<ImplicitCastExpr>(E))
+ E = C->getSubExpr();
+
+ // Remove call to conversion op.
+ if (CXXMemberCallExpr *MCE = dyn_cast<CXXMemberCallExpr>(E)) {
+ if (isa<CXXConversionDecl>(MCE->getMethodDecl()))
+ E = MCE->getImplicitObjectArgument();
+ }
+
+ if (BinaryOperator *OP = dyn_cast<BinaryOperator>(E)) {
+ if (IsArithmeticOp(OP->getOpcode())) {
+ *Opcode = OP->getOpcode();
+ *RHS = OP->getRHS();
+ return true;
+ }
+ }
+
+ if (CXXOperatorCallExpr* Call = dyn_cast<CXXOperatorCallExpr>(E)) {
+ OverloadedOperatorKind OO = Call->getOperator();
+ if (OO >= OO_Plus && OO <= OO_Arrow) {
+ if (Call->getNumArgs() == 2) {
+ *Opcode = BinaryOperator::getOverloadedOpcode(OO);
+ if (IsArithmeticOp(*Opcode)) {
+ *RHS = Call->getArg(1);
+ return true;
+ }
+ }
+ }
+ }
+
+ return false;
+}
+
static bool IsLogicOp(BinaryOperatorKind Opc) {
return (Opc >= BO_LT && Opc <= BO_NE) || (Opc >= BO_LAnd && Opc <= BO_LOr);
}
+/// Returns true if E looks boolean, i.e. has a boolean type or is a logical
+/// expression.
+static bool ExprLooksBoolean(Expr *E) {
+ // Remove implicit casts.
+ while (ImplicitCastExpr *C = dyn_cast<ImplicitCastExpr>(E))
+ E = C->getSubExpr();
+
+ // Remove parentheses.
+ while (ParenExpr *P = dyn_cast<ParenExpr>(E))
+ E = P->getSubExpr();
+
+ if (E->getType()->isBooleanType())
+ return true;
+ else if (BinaryOperator *OP = dyn_cast<BinaryOperator>(E))
+ return IsLogicOp(OP->getOpcode());
+ else if (UnaryOperator *OP = dyn_cast<UnaryOperator>(E))
+ return OP->getOpcode() == UO_LNot;
+
+ return false;
+}
+
/// 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:
@@ -6243,52 +6303,36 @@ static void DiagnoseConditionalPrecedence(Sema &Self,
Expr *cond,
Expr *lhs,
Expr *rhs) {
- while (ImplicitCastExpr *C = dyn_cast<ImplicitCastExpr>(cond))
- cond = C->getSubExpr();
+ BinaryOperatorKind CondOpcode;
+ Expr *CondRHS;
- if (BinaryOperator *OP = dyn_cast<BinaryOperator>(cond)) {
- if (!IsArithmeticOp(OP->getOpcode()))
- return;
-
- // Drill down on the RHS of the condition.
- Expr *CondRHS = OP->getRHS();
- while (ImplicitCastExpr *C = dyn_cast<ImplicitCastExpr>(CondRHS))
- CondRHS = C->getSubExpr();
- while (ParenExpr *P = dyn_cast<ParenExpr>(CondRHS))
- CondRHS = P->getSubExpr();
+ if (!IsArithmeticBinaryExpr(cond, &CondOpcode, &CondRHS))
+ return;
+ if (!ExprLooksBoolean(CondRHS))
+ return;
- bool CondRHSLooksBoolean = false;
- if (CondRHS->getType()->isBooleanType())
- CondRHSLooksBoolean = true;
- else if (BinaryOperator *CondRHSOP = dyn_cast<BinaryOperator>(CondRHS))
- CondRHSLooksBoolean = IsLogicOp(CondRHSOP->getOpcode());
- else if (UnaryOperator *CondRHSOP = dyn_cast<UnaryOperator>(CondRHS))
- CondRHSLooksBoolean = CondRHSOP->getOpcode() == UO_LNot;
+ // The condition is an arithmetic binary expression, with a right-
+ // hand side that looks boolean, so warn.
- if (CondRHSLooksBoolean) {
- // The condition is an arithmetic binary expression, with a right-
- // hand side that looks boolean, so warn.
+ PartialDiagnostic Warn = Self.PDiag(diag::warn_precedence_conditional)
+ << cond->getSourceRange()
+ << BinaryOperator::getOpcodeStr(CondOpcode);
- PartialDiagnostic Warn = Self.PDiag(diag::warn_precedence_conditional)
- << OP->getSourceRange()
- << BinaryOperator::getOpcodeStr(OP->getOpcode());
+ PartialDiagnostic FirstNote =
+ Self.PDiag(diag::note_precedence_conditional_silence)
+ << BinaryOperator::getOpcodeStr(CondOpcode);
- PartialDiagnostic FirstNote =
- Self.PDiag(diag::note_precedence_conditional_silence)
- << BinaryOperator::getOpcodeStr(OP->getOpcode());
+ SourceRange FirstParenRange(cond->getLocStart(),
+ cond->getLocEnd());
- SourceRange FirstParenRange(OP->getLHS()->getLocStart(),
- OP->getRHS()->getLocEnd());
+ PartialDiagnostic SecondNote =
+ Self.PDiag(diag::note_precedence_conditional_first);
- PartialDiagnostic SecondNote =
- Self.PDiag(diag::note_precedence_conditional_first);
- SourceRange SecondParenRange(OP->getRHS()->getLocStart(),
- rhs->getLocEnd());
+ SourceRange SecondParenRange(CondRHS->getLocStart(),
+ rhs->getLocEnd());
- SuggestParentheses(Self, OpLoc, Warn, FirstNote, FirstParenRange,
- SecondNote, SecondParenRange);
- }
- }
+ SuggestParentheses(Self, OpLoc, Warn, FirstNote, FirstParenRange,
+ SecondNote, SecondParenRange);
}
/// ActOnConditionalOp - Parse a ?: operation. Note that 'LHS' may be null
diff --git a/test/Sema/parentheses.cpp b/test/Sema/parentheses.cpp
index ad1f399..a25f2a0 100644
--- a/test/Sema/parentheses.cpp
+++ b/test/Sema/parentheses.cpp
@@ -16,3 +16,16 @@ void conditional_op(int x, int y, bool b) {
// expected-note {{place parentheses around the ?: expression to evaluate it first}} \
// expected-note {{place parentheses around the * expression to silence this warning}}
}
+
+class Stream {
+public:
+ operator int();
+ Stream &operator<<(int);
+ Stream &operator<<(const char*);
+};
+
+void f(Stream& s, bool b) {
+ (void)(s << b ? "foo" : "bar"); // expected-warning {{?: has lower precedence than <<}} \
+ // expected-note {{place parentheses around the ?: expression to evaluate it first}} \
+ // expected-note {{place parentheses around the << expression to silence this warning}}
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
