From: Richard Trieu Date: Tue, 11 Mar 2014 03:11:08 +0000 (+0000) Subject: Move the warning about unused relational comparison from -Wunused-value to X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=61e100da3ce83aa0186b080c1c772e09dc848995;p=clang Move the warning about unused relational comparison from -Wunused-value to -Wunused-comparison. Also, newly warn on unused result from overloaded relational comparisons, now also in -Wunused-comparison. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@203535 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 7f2abe6452..a4de7f0873 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -5813,7 +5813,7 @@ def warn_unused_volatile : Warning< InGroup>; def warn_unused_comparison : Warning< - "%select{equality|inequality}0 comparison result unused">, + "%select{%select{|in}1equality|relational}0 comparison result unused">, InGroup; def note_inequality_comparison_to_or_assign : Note< "use '|=' to turn this inequality comparison into an or-assignment">; diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp index 0b8948e5fa..2f5f14f38e 100644 --- a/lib/AST/Expr.cpp +++ b/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(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/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp index d726f5f16c..2c6dd50e04 100644 --- a/lib/Sema/SemaStmt.cpp +++ b/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(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(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/test/Sema/unused-expr.c b/test/Sema/unused-expr.c index bb7882de40..43d8150b02 100644 --- a/test/Sema/unused-expr.c +++ b/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/test/SemaCXX/MicrosoftExtensions.cpp b/test/SemaCXX/MicrosoftExtensions.cpp index 6f90b6ae7c..9f7bcc625f 100644 --- a/test/SemaCXX/MicrosoftExtensions.cpp +++ b/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/test/SemaCXX/warn-unused-comparison.cpp b/test/SemaCXX/warn-unused-comparison.cpp index 0153f213ba..8e6f0f49f1 100644 --- a/test/SemaCXX/warn-unused-comparison.cpp +++ b/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/test/SemaTemplate/resolve-single-template-id.cpp b/test/SemaTemplate/resolve-single-template-id.cpp index 915c42c85a..bebca76c4f 100644 --- a/test/SemaTemplate/resolve-single-template-id.cpp +++ b/test/SemaTemplate/resolve-single-template-id.cpp @@ -67,10 +67,10 @@ int main() b = (void (*)()) twoT; one < one; //expected-warning {{self-comparison always evaluates to false}} \ - //expected-warning {{expression result unused}} + //expected-warning {{relational comparison result unused}} oneT < oneT; //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 < twoT; //expected-error {{reference to overloaded function could not be resolved; did you mean to call it?}} {{cannot resolve overloaded function 'twoT' from context}}