diff options
| -rw-r--r-- | clang/include/clang/Basic/DiagnosticSemaKinds.td | 2 | ||||
| -rw-r--r-- | clang/lib/AST/Expr.cpp | 15 | ||||
| -rw-r--r-- | clang/lib/Sema/SemaStmt.cpp | 27 | ||||
| -rw-r--r-- | clang/test/Sema/unused-expr.c | 18 | ||||
| -rw-r--r-- | clang/test/SemaCXX/MicrosoftExtensions.cpp | 2 | ||||
| -rw-r--r-- | clang/test/SemaCXX/warn-unused-comparison.cpp | 14 | ||||
| -rw-r--r-- | clang/test/SemaTemplate/resolve-single-template-id.cpp | 4 |
7 files changed, 58 insertions, 24 deletions
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 7f2abe64524..a4de7f08736 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -5813,7 +5813,7 @@ def warn_unused_volatile : Warning< InGroup<DiagGroup<"unused-volatile-lvalue">>; def warn_unused_comparison : Warning< - "%select{equality|inequality}0 comparison result unused">, + "%select{%select{|in}1equality|relational}0 comparison result unused">, InGroup<UnusedComparison>; def note_inequality_comparison_to_or_assign : Note< "use '|=' to turn this inequality comparison into an or-assignment">; diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index 0b8948e5fa4..2f5f14f38e3 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -2064,15 +2064,22 @@ bool Expr::isUnusedResultAWarning(const Expr *&WarnE, SourceLocation &Loc, return true; case CXXOperatorCallExprClass: { - // We warn about operator== and operator!= even when user-defined operator + // Warn about operator ==,!=,<,>,<=, and >= even when user-defined operator // overloads as there is no reasonable way to define these such that they // have non-trivial, desirable side-effects. See the -Wunused-comparison - // warning: these operators are commonly typo'ed, and so warning on them + // warning: operators == and != are commonly typo'ed, and so warning on them // provides additional value as well. If this list is updated, // DiagnoseUnusedComparison should be as well. const CXXOperatorCallExpr *Op = cast<CXXOperatorCallExpr>(this); - if (Op->getOperator() == OO_EqualEqual || - Op->getOperator() == OO_ExclaimEqual) { + switch (Op->getOperator()) { + default: + break; + case OO_EqualEqual: + case OO_ExclaimEqual: + case OO_Less: + case OO_Greater: + case OO_GreaterEqual: + case OO_LessEqual: WarnE = this; Loc = Op->getOperatorLoc(); R1 = Op->getSourceRange(); diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index d726f5f16ca..2c6dd50e04e 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -114,25 +114,38 @@ void Sema::ActOnForEachDeclStmt(DeclGroupPtrTy dg) { } } -/// \brief Diagnose unused '==' and '!=' as likely typos for '=' or '|='. +/// \brief Diagnose unused comparisons, both builtin and overloaded operators. +/// For '==' and '!=', suggest fixits for '=' or '|='. /// /// Adding a cast to void (or other expression wrappers) will prevent the /// warning from firing. static bool DiagnoseUnusedComparison(Sema &S, const Expr *E) { SourceLocation Loc; - bool IsNotEqual, CanAssign; + bool IsNotEqual, CanAssign, IsRelational; if (const BinaryOperator *Op = dyn_cast<BinaryOperator>(E)) { - if (Op->getOpcode() != BO_EQ && Op->getOpcode() != BO_NE) + if (!Op->isComparisonOp()) return false; + IsRelational = Op->isRelationalOp(); Loc = Op->getOperatorLoc(); IsNotEqual = Op->getOpcode() == BO_NE; CanAssign = Op->getLHS()->IgnoreParenImpCasts()->isLValue(); } else if (const CXXOperatorCallExpr *Op = dyn_cast<CXXOperatorCallExpr>(E)) { - if (Op->getOperator() != OO_EqualEqual && - Op->getOperator() != OO_ExclaimEqual) + switch (Op->getOperator()) { + default: return false; + case OO_EqualEqual: + case OO_ExclaimEqual: + IsRelational = false; + break; + case OO_Less: + case OO_Greater: + case OO_GreaterEqual: + case OO_LessEqual: + IsRelational = true; + break; + } Loc = Op->getOperatorLoc(); IsNotEqual = Op->getOperator() == OO_ExclaimEqual; @@ -148,11 +161,11 @@ static bool DiagnoseUnusedComparison(Sema &S, const Expr *E) { return false; S.Diag(Loc, diag::warn_unused_comparison) - << (unsigned)IsNotEqual << E->getSourceRange(); + << (unsigned)IsRelational << (unsigned)IsNotEqual << E->getSourceRange(); // If the LHS is a plausible entity to assign to, provide a fixit hint to // correct common typos. - if (CanAssign) { + if (!IsRelational && CanAssign) { if (IsNotEqual) S.Diag(Loc, diag::note_inequality_comparison_to_or_assign) << FixItHint::CreateReplacement(Loc, "|="); diff --git a/clang/test/Sema/unused-expr.c b/clang/test/Sema/unused-expr.c index bb7882de40f..43d8150b02c 100644 --- a/clang/test/Sema/unused-expr.c +++ b/clang/test/Sema/unused-expr.c @@ -7,11 +7,11 @@ double sqrt(double X); // implicitly const because of no -fmath-errno! void bar(volatile int *VP, int *P, int A, _Complex double C, volatile _Complex double VC) { - VP < P; // expected-warning {{expression result unused}} + VP < P; // expected-warning {{relational comparison result unused}} (void)A; (void)foo(1,2); // no warning. - A < foo(1, 2); // expected-warning {{expression result unused}} + A < foo(1, 2); // expected-warning {{relational comparison result unused}} foo(1,2)+foo(4,3); // expected-warning {{expression result unused}} @@ -54,23 +54,23 @@ void t4(int a) { int b = 0; if (a) - b < 1; // expected-warning{{expression result unused}} + b < 1; // expected-warning{{relational comparison result unused}} else - b < 2; // expected-warning{{expression result unused}} + b < 2; // expected-warning{{relational comparison result unused}} while (1) - b < 3; // expected-warning{{expression result unused}} + b < 3; // expected-warning{{relational comparison result unused}} do - b < 4; // expected-warning{{expression result unused}} + b < 4; // expected-warning{{relational comparison result unused}} while (1); for (;;) - b < 5; // expected-warning{{expression result unused}} + b < 5; // expected-warning{{relational comparison result unused}} - for (b < 1;;) {} // expected-warning{{expression result unused}} + for (b < 1;;) {} // expected-warning{{relational comparison result unused}} for (;b < 1;) {} - for (;;b < 1) {} // expected-warning{{expression result unused}} + for (;;b < 1) {} // expected-warning{{relational comparison result unused}} } // rdar://7186119 diff --git a/clang/test/SemaCXX/MicrosoftExtensions.cpp b/clang/test/SemaCXX/MicrosoftExtensions.cpp index 6f90b6ae7c4..9f7bcc625f4 100644 --- a/clang/test/SemaCXX/MicrosoftExtensions.cpp +++ b/clang/test/SemaCXX/MicrosoftExtensions.cpp @@ -303,7 +303,7 @@ struct SP9 { __declspec(property(get=GetV, put=SetV)) T V; T GetV() { return 0; } void SetV(T v) {} - void f() { V = this->V; V < this->V; } + bool f() { V = this->V; return V < this->V; } void g() { V++; } void h() { V*=2; } }; diff --git a/clang/test/SemaCXX/warn-unused-comparison.cpp b/clang/test/SemaCXX/warn-unused-comparison.cpp index 0153f213ba1..8e6f0f49f19 100644 --- a/clang/test/SemaCXX/warn-unused-comparison.cpp +++ b/clang/test/SemaCXX/warn-unused-comparison.cpp @@ -3,6 +3,10 @@ struct A { bool operator==(const A&); bool operator!=(const A&); + bool operator<(const A&); + bool operator>(const A&); + bool operator<=(const A&); + bool operator>=(const A&); A operator|=(const A&); operator bool(); }; @@ -15,6 +19,11 @@ void test() { // expected-note {{use '=' to turn this equality comparison into an assignment}} x != 7; // expected-warning {{inequality comparison result unused}} \ // expected-note {{use '|=' to turn this inequality comparison into an or-assignment}} + x < 7; // expected-warning {{relational comparison result unused}} + x > 7; // expected-warning {{relational comparison result unused}} + x <= 7; // expected-warning {{relational comparison result unused}} + x >= 7; // expected-warning {{relational comparison result unused}} + 7 == x; // expected-warning {{equality comparison result unused}} p == p; // expected-warning {{equality comparison result unused}} \ // expected-note {{use '=' to turn this equality comparison into an assignment}} \ @@ -25,6 +34,11 @@ void test() { // expected-note {{use '=' to turn this equality comparison into an assignment}} a != b; // expected-warning {{inequality comparison result unused}} \ // expected-note {{use '|=' to turn this inequality comparison into an or-assignment}} + a < b; // expected-warning {{relational comparison result unused}} + a > b; // expected-warning {{relational comparison result unused}} + a <= b; // expected-warning {{relational comparison result unused}} + a >= b; // expected-warning {{relational comparison result unused}} + A() == b; // expected-warning {{equality comparison result unused}} if (42) x == 7; // expected-warning {{equality comparison result unused}} \ // expected-note {{use '=' to turn this equality comparison into an assignment}} diff --git a/clang/test/SemaTemplate/resolve-single-template-id.cpp b/clang/test/SemaTemplate/resolve-single-template-id.cpp index 915c42c85a6..bebca76c4f6 100644 --- a/clang/test/SemaTemplate/resolve-single-template-id.cpp +++ b/clang/test/SemaTemplate/resolve-single-template-id.cpp @@ -67,10 +67,10 @@ int main() b = (void (*)()) twoT<int>; one < one; //expected-warning {{self-comparison always evaluates to false}} \ - //expected-warning {{expression result unused}} + //expected-warning {{relational comparison result unused}} oneT<int> < oneT<int>; //expected-warning {{self-comparison always evaluates to false}} \ - //expected-warning {{expression result unused}} + //expected-warning {{relational comparison result unused}} two < two; //expected-error 2 {{reference to overloaded function could not be resolved; did you mean to call it with no arguments?}} expected-error {{invalid operands to binary expression ('void' and 'void')}} twoT<int> < twoT<int>; //expected-error {{reference to overloaded function could not be resolved; did you mean to call it?}} {{cannot resolve overloaded function 'twoT' from context}} |
