[cfe-commits] r132784 - in /cfe/trunk: include/clang/AST/Expr.h lib/AST/Expr.cpp lib/Sema/SemaExpr.cpp test/Sema/parentheses.cpp
Hans Wennborg
hans at hanshq.net
Thu Jun 9 10:06:51 PDT 2011
Author: hans
Date: Thu Jun 9 12:06:51 2011
New Revision: 132784
URL: http://llvm.org/viewvc/llvm-project?rev=132784&view=rev
Log:
Handle overloaded operators in ?: precedence warning
This is a follow-up to r132565, and should address the rest of PR9969:
Warn about cases such as
int foo(A a, bool b) {
return a + b ? 1 : 2; // user probably meant a + (b ? 1 : 2);
}
also when + is an overloaded operator call.
Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/parentheses.cpp
Modified: cfe/trunk/include/clang/AST/Expr.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=132784&r1=132783&r2=132784&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Thu Jun 9 12:06:51 2011
@@ -515,6 +515,14 @@
/// ParenExpr or ImplicitCastExprs, returning their operand.
Expr *IgnoreParenImpCasts();
+ /// IgnoreConversionOperator - Ignore conversion operator. If this Expr is a
+ /// call to a conversion operator, return the argument.
+ Expr *IgnoreConversionOperator();
+
+ const Expr *IgnoreConversionOperator() const {
+ return const_cast<Expr*>(this)->IgnoreConversionOperator();
+ }
+
const Expr *IgnoreParenImpCasts() const {
return const_cast<Expr*>(this)->IgnoreParenImpCasts();
}
Modified: cfe/trunk/lib/AST/Expr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=132784&r1=132783&r2=132784&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Expr.cpp (original)
+++ cfe/trunk/lib/AST/Expr.cpp Thu Jun 9 12:06:51 2011
@@ -1988,6 +1988,14 @@
}
}
+Expr *Expr::IgnoreConversionOperator() {
+ if (CXXMemberCallExpr *MCE = dyn_cast<CXXMemberCallExpr>(this)) {
+ if (isa<CXXConversionDecl>(MCE->getMethodDecl()))
+ return MCE->getImplicitObjectArgument();
+ }
+ return this;
+}
+
/// IgnoreParenNoopCasts - Ignore parentheses and casts that do not change the
/// value (including ptr->int casts of the same size). Strip off any
/// ParenExpr or CastExprs, returning their operand.
@@ -3023,4 +3031,3 @@
ExprBits.TypeDependent = TypeDependent;
ExprBits.ValueDependent = ValueDependent;
}
-
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=132784&r1=132783&r2=132784&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Jun 9 12:06:51 2011
@@ -6230,10 +6230,66 @@
return Opc >= BO_Mul && Opc <= BO_Shr;
}
+/// IsArithmeticBinaryExpr - 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) {
+ E = E->IgnoreParenImpCasts();
+ E = E->IgnoreConversionOperator();
+ E = E->IgnoreParenImpCasts();
+
+ // Built-in binary operator.
+ if (BinaryOperator *OP = dyn_cast<BinaryOperator>(E)) {
+ if (IsArithmeticOp(OP->getOpcode())) {
+ *Opcode = OP->getOpcode();
+ *RHS = OP->getRHS();
+ return true;
+ }
+ }
+
+ // Overloaded operator.
+ if (CXXOperatorCallExpr *Call = dyn_cast<CXXOperatorCallExpr>(E)) {
+ if (Call->getNumArgs() != 2)
+ return false;
+
+ // Make sure this is really a binary operator that is safe to pass into
+ // BinaryOperator::getOverloadedOpcode(), e.g. it's not a subscript op.
+ OverloadedOperatorKind OO = Call->getOperator();
+ if (OO < OO_Plus || OO > OO_Arrow)
+ return false;
+
+ BinaryOperatorKind OpKind = BinaryOperator::getOverloadedOpcode(OO);
+ if (IsArithmeticOp(OpKind)) {
+ *Opcode = OpKind;
+ *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);
}
+/// ExprLooksBoolean - Returns true if E looks boolean, i.e. it has boolean type
+/// or is a logical expression such as (x==y) which has int type, but is
+/// commonly interpreted as boolean.
+static bool ExprLooksBoolean(Expr *E) {
+ E = E->IgnoreParenImpCasts();
+
+ if (E->getType()->isBooleanType())
+ return true;
+ if (BinaryOperator *OP = dyn_cast<BinaryOperator>(E))
+ return IsLogicOp(OP->getOpcode());
+ 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 +6299,36 @@
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;
+ if (!IsArithmeticBinaryExpr(cond, &CondOpcode, &CondRHS))
+ return;
+ if (!ExprLooksBoolean(CondRHS))
+ 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();
-
- 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;
-
- 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)
- << OP->getSourceRange()
- << BinaryOperator::getOpcodeStr(OP->getOpcode());
-
- PartialDiagnostic FirstNote =
- Self.PDiag(diag::note_precedence_conditional_silence)
- << BinaryOperator::getOpcodeStr(OP->getOpcode());
-
- SourceRange FirstParenRange(OP->getLHS()->getLocStart(),
- OP->getRHS()->getLocEnd());
-
- PartialDiagnostic SecondNote =
- Self.PDiag(diag::note_precedence_conditional_first);
- SourceRange SecondParenRange(OP->getRHS()->getLocStart(),
- rhs->getLocEnd());
+ // The condition is an arithmetic binary expression, with a right-
+ // hand side that looks boolean, so warn.
- SuggestParentheses(Self, OpLoc, Warn, FirstNote, FirstParenRange,
- SecondNote, SecondParenRange);
- }
- }
+ PartialDiagnostic Warn = Self.PDiag(diag::warn_precedence_conditional)
+ << cond->getSourceRange()
+ << BinaryOperator::getOpcodeStr(CondOpcode);
+
+ PartialDiagnostic FirstNote =
+ Self.PDiag(diag::note_precedence_conditional_silence)
+ << BinaryOperator::getOpcodeStr(CondOpcode);
+
+ SourceRange FirstParenRange(cond->getLocStart(),
+ cond->getLocEnd());
+
+ PartialDiagnostic SecondNote =
+ Self.PDiag(diag::note_precedence_conditional_first);
+
+ SourceRange SecondParenRange(CondRHS->getLocStart(),
+ rhs->getLocEnd());
+
+ SuggestParentheses(Self, OpLoc, Warn, FirstNote, FirstParenRange,
+ SecondNote, SecondParenRange);
}
/// ActOnConditionalOp - Parse a ?: operation. Note that 'LHS' may be null
Modified: cfe/trunk/test/Sema/parentheses.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/parentheses.cpp?rev=132784&r1=132783&r2=132784&view=diff
==============================================================================
--- cfe/trunk/test/Sema/parentheses.cpp (original)
+++ cfe/trunk/test/Sema/parentheses.cpp Thu Jun 9 12:06:51 2011
@@ -16,3 +16,16 @@
// 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}}
+}
More information about the cfe-commits
mailing list