From e89ca5f7d2e856d9c489dfba0003130784c18ac8 Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Wed, 17 Aug 2011 08:38:11 +0000 Subject: [PATCH] Don't suggest assignment in implausible situation. We still warn, as the code is very likely to be buggy, but its going to require more significant changes on the part of the user to correct it in this case. llvm-svn: 137820 --- clang/lib/Sema/SemaStmt.cpp | 24 ++++++++++++------- .../SemaCXX/warn-top-level-comparison.cpp | 4 ++++ 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index 79ae67210f2b..79e317bb8caa 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -147,21 +147,23 @@ static void DiagnoseTopLevelComparison(Sema &S, const Stmt *Statement) { } SourceLocation Loc; - bool IsNotEqual = false; + bool IsNotEqual, CanAssign; if (const BinaryOperator *Op = dyn_cast(E)) { if (Op->getOpcode() != BO_EQ && Op->getOpcode() != BO_NE) return; - IsNotEqual = Op->getOpcode() == BO_NE; 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) return; - IsNotEqual = Op->getOperator() == OO_ExclaimEqual; Loc = Op->getOperatorLoc(); + IsNotEqual = Op->getOperator() == OO_ExclaimEqual; + CanAssign = Op->getArg(0)->IgnoreParenImpCasts()->isLValue(); } else { // Not a typo-prone comparison. return; @@ -181,12 +183,16 @@ static void DiagnoseTopLevelComparison(Sema &S, const Stmt *Statement) { << FixItHint::CreateInsertion(Open, "(void)(") << FixItHint::CreateInsertion(Close, ")"); - if (IsNotEqual) - S.Diag(Loc, diag::note_inequality_comparison_to_or_assign) - << FixItHint::CreateReplacement(Loc, "|="); - else - S.Diag(Loc, diag::note_equality_comparison_to_assign) - << FixItHint::CreateReplacement(Loc, "="); + // If the LHS is a plausible entity to assign to, provide a fixit hint to + // correct common typos. + if (CanAssign) { + if (IsNotEqual) + S.Diag(Loc, diag::note_inequality_comparison_to_or_assign) + << FixItHint::CreateReplacement(Loc, "|="); + else + S.Diag(Loc, diag::note_equality_comparison_to_assign) + << FixItHint::CreateReplacement(Loc, "="); + } } void Sema::DiagnoseUnusedExprResult(const Stmt *S) { diff --git a/clang/test/SemaCXX/warn-top-level-comparison.cpp b/clang/test/SemaCXX/warn-top-level-comparison.cpp index e298d2653b3b..dad21aeedc08 100644 --- a/clang/test/SemaCXX/warn-top-level-comparison.cpp +++ b/clang/test/SemaCXX/warn-top-level-comparison.cpp @@ -17,6 +17,8 @@ void test() { x != 7; // expected-warning {{inequality comparison as an unused top-level statement}} \ // expected-note {{cast this comparison to void to silence this warning}} \ // expected-note {{use '|=' to turn this inequality comparison into an or-assignment}} + 7 == x; // expected-warning {{equality comparison as an unused top-level statement}} \ + // expected-note {{cast this comparison to void to silence this warning}} p == p; // expected-warning {{equality comparison as an unused top-level statement}} \ // expected-note {{cast this comparison to void to silence this warning}} \ // expected-note {{use '=' to turn this equality comparison into an assignment}} \ @@ -30,6 +32,8 @@ void test() { a != b; // expected-warning {{inequality comparison as an unused top-level statement}} \ // expected-note {{cast this comparison to void to silence this warning}} \ // expected-note {{use '|=' to turn this inequality comparison into an or-assignment}} + A() == b; // expected-warning {{equality comparison as an unused top-level statement}} \ + // expected-note {{cast this comparison to void to silence this warning}} if (42) x == 7; // expected-warning {{equality comparison as an unused top-level statement}} \ // expected-note {{cast this comparison to void to silence this warning}} \ // expected-note {{use '=' to turn this equality comparison into an assignment}} -- GitLab