summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--clang/include/clang/Basic/DiagnosticGroups.td2
-rw-r--r--clang/include/clang/Basic/DiagnosticSemaKinds.td8
-rw-r--r--clang/lib/Sema/SemaExpr.cpp40
-rw-r--r--clang/test/SemaCXX/warn-logical-not-compare.cpp112
4 files changed, 162 insertions, 0 deletions
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 8259e2e3680..99b342454c4 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -136,6 +136,7 @@ def FourByteMultiChar : DiagGroup<"four-char-constants">;
def GlobalConstructors : DiagGroup<"global-constructors">;
def BitwiseOpParentheses: DiagGroup<"bitwise-op-parentheses">;
def LogicalOpParentheses: DiagGroup<"logical-op-parentheses">;
+def LogicalNotParentheses: DiagGroup<"logical-not-parentheses">;
def ShiftOpParentheses: DiagGroup<"shift-op-parentheses">;
def OverloadedShiftOpParentheses: DiagGroup<"overloaded-shift-op-parentheses">;
def DanglingElse: DiagGroup<"dangling-else">;
@@ -368,6 +369,7 @@ def DuplicateArgDecl : DiagGroup<"duplicate-method-arg">;
def ParenthesesOnEquality : DiagGroup<"parentheses-equality">;
def Parentheses : DiagGroup<"parentheses",
[LogicalOpParentheses,
+ LogicalNotParentheses,
BitwiseOpParentheses,
ShiftOpParentheses,
OverloadedShiftOpParentheses,
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index f79233e52e1..b9c0518dece 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4435,6 +4435,14 @@ def warn_null_in_comparison_operation : Warning<
"%select{(%1 and NULL)|(NULL and %1)}0">,
InGroup<NullArithmetic>;
+def warn_logical_not_on_lhs_of_comparison : Warning<
+ "logical not is only applied to the left hand side of this comparison">,
+ InGroup<LogicalNotParentheses>;
+def note_logical_not_fix : Note<
+ "add parentheses after the '!' to evaluate the comparison first">;
+def note_logical_not_silence_with_parens : Note<
+ "add parentheses around left hand side expression to silence this warning">;
+
def err_invalid_this_use : Error<
"invalid use of 'this' outside of a non-static member function">;
def err_this_static_member_func : Error<
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 610dc28e3d9..f3fc1139544 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -7250,6 +7250,45 @@ static void diagnoseObjCLiteralComparison(Sema &S, SourceLocation Loc,
}
}
+static void diagnoseLogicalNotOnLHSofComparison(Sema &S, ExprResult &LHS,
+ ExprResult &RHS,
+ SourceLocation Loc,
+ unsigned OpaqueOpc) {
+ // This checking requires bools.
+ if (!S.getLangOpts().Bool) return;
+
+ // Check that left hand side is !something.
+ UnaryOperator *UO = dyn_cast<UnaryOperator>(LHS.get());
+ if (!UO || UO->getOpcode() != UO_LNot) return;
+
+ // Only check if the right hand side is non-bool arithmetic type.
+ if (RHS.get()->getType()->isBooleanType()) return;
+
+ // Make sure that the something in !something is not bool.
+ Expr *SubExpr = UO->getSubExpr()->IgnoreImpCasts();
+ if (SubExpr->getType()->isBooleanType()) return;
+
+ // Emit warning.
+ S.Diag(UO->getOperatorLoc(), diag::warn_logical_not_on_lhs_of_comparison)
+ << Loc;
+
+ // First note suggest !(x < y)
+ SourceLocation FirstOpen = SubExpr->getLocStart();
+ SourceLocation FirstClose = RHS.get()->getLocEnd();
+ FirstClose = S.getPreprocessor().getLocForEndOfToken(FirstClose);
+ S.Diag(UO->getOperatorLoc(), diag::note_logical_not_fix)
+ << FixItHint::CreateInsertion(FirstOpen, "(")
+ << FixItHint::CreateInsertion(FirstClose, ")");
+
+ // Second note suggests (!x) < y
+ SourceLocation SecondOpen = LHS.get()->getLocStart();
+ SourceLocation SecondClose = LHS.get()->getLocEnd();
+ SecondClose = S.getPreprocessor().getLocForEndOfToken(SecondClose);
+ S.Diag(UO->getOperatorLoc(), diag::note_logical_not_silence_with_parens)
+ << FixItHint::CreateInsertion(SecondOpen, "(")
+ << FixItHint::CreateInsertion(SecondClose, ")");
+}
+
// C99 6.5.8, C++ [expr.rel]
QualType Sema::CheckCompareOperands(ExprResult &LHS, ExprResult &RHS,
SourceLocation Loc, unsigned OpaqueOpc,
@@ -7270,6 +7309,7 @@ QualType Sema::CheckCompareOperands(ExprResult &LHS, ExprResult &RHS,
Expr *RHSStripped = RHS.get()->IgnoreParenImpCasts();
checkEnumComparison(*this, Loc, LHS.get(), RHS.get());
+ diagnoseLogicalNotOnLHSofComparison(*this, LHS, RHS, Loc, OpaqueOpc);
if (!LHSType->hasFloatingRepresentation() &&
!(LHSType->isBlockPointerType() && IsRelational) &&
diff --git a/clang/test/SemaCXX/warn-logical-not-compare.cpp b/clang/test/SemaCXX/warn-logical-not-compare.cpp
new file mode 100644
index 00000000000..365a0285118
--- /dev/null
+++ b/clang/test/SemaCXX/warn-logical-not-compare.cpp
@@ -0,0 +1,112 @@
+// RUN: %clang_cc1 -fsyntax-only -Wlogical-not-parentheses -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wlogical-not-parentheses -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+bool getBool();
+int getInt();
+
+bool test1(int i1, int i2, bool b1, bool b2) {
+ bool ret;
+
+ ret = !i1 == i2;
+ // expected-warning@-1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note@-2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note@-3 {{add parentheses around left hand side expression to silence this warning}}
+ // CHECK: to evaluate the comparison first
+ // CHECK: fix-it:"{{.*}}":{10:10-10:10}:"("
+ // CHECK: fix-it:"{{.*}}":{10:18-10:18}:")"
+ // CHECK: to silence this warning
+ // CHECK: fix-it:"{{.*}}":{10:9-10:9}:"("
+ // CHECK: fix-it:"{{.*}}":{10:12-10:12}:")"
+
+ ret = !i1 != i2;
+ //expected-warning@-1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note@-2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note@-3 {{add parentheses around left hand side expression to silence this warning}}
+ // CHECK: to evaluate the comparison first
+ // CHECK: fix-it:"{{.*}}":{21:10-21:10}:"("
+ // CHECK: fix-it:"{{.*}}":{21:18-21:18}:")"
+ // CHECK: to silence this warning
+ // CHECK: fix-it:"{{.*}}":{21:9-21:9}:"("
+ // CHECK: fix-it:"{{.*}}":{21:12-21:12}:")"
+
+ ret = !i1 < i2;
+ //expected-warning@-1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note@-2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note@-3 {{add parentheses around left hand side expression to silence this warning}}
+ // CHECK: to evaluate the comparison first
+ // CHECK: fix-it:"{{.*}}":{32:10-32:10}:"("
+ // CHECK: fix-it:"{{.*}}":{32:17-32:17}:")"
+ // CHECK: to silence this warning
+ // CHECK: fix-it:"{{.*}}":{32:9-32:9}:"("
+ // CHECK: fix-it:"{{.*}}":{32:12-32:12}:")"
+
+ ret = !i1 > i2;
+ //expected-warning@-1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note@-2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note@-3 {{add parentheses around left hand side expression to silence this warning}}
+ // CHECK: to evaluate the comparison first
+ // CHECK: fix-it:"{{.*}}":{43:10-43:10}:"("
+ // CHECK: fix-it:"{{.*}}":{43:17-43:17}:")"
+ // CHECK: to silence this warning
+ // CHECK: fix-it:"{{.*}}":{43:9-43:9}:"("
+ // CHECK: fix-it:"{{.*}}":{43:12-43:12}:")"
+
+ ret = !i1 <= i2;
+ //expected-warning@-1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note@-2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note@-3 {{add parentheses around left hand side expression to silence this warning}}
+ // CHECK: to evaluate the comparison first
+ // CHECK: fix-it:"{{.*}}":{54:10-54:10}:"("
+ // CHECK: fix-it:"{{.*}}":{54:18-54:18}:")"
+ // CHECK: to silence this warning
+ // CHECK: fix-it:"{{.*}}":{54:9-54:9}:"("
+ // CHECK: fix-it:"{{.*}}":{54:12-54:12}:")"
+
+ ret = !i1 >= i2;
+ //expected-warning@-1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note@-2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note@-3 {{add parentheses around left hand side expression to silence this warning}}
+ // CHECK: to evaluate the comparison first
+ // CHECK: fix-it:"{{.*}}":{65:10-65:10}:"("
+ // CHECK: fix-it:"{{.*}}":{65:18-65:18}:")"
+ // CHECK: to silence this warning
+ // CHECK: fix-it:"{{.*}}":{65:9-65:9}:"("
+ // CHECK: fix-it:"{{.*}}":{65:12-65:12}:")"
+
+ ret = i1 == i2;
+ ret = i1 != i2;
+ ret = i1 < i2;
+ ret = i1 > i2;
+ ret = i1 <= i2;
+ ret = i1 >= i2;
+
+ // Warning silenced by parens.
+ ret = (!i1) == i2;
+ ret = (!i1) != i2;
+ ret = (!i1) < i2;
+ ret = (!i1) > i2;
+ ret = (!i1) <= i2;
+ ret = (!i1) >= i2;
+
+ ret = !b1 == b2;
+ ret = !b1 != b2;
+ ret = !b1 < b2;
+ ret = !b1 > b2;
+ ret = !b1 <= b2;
+ ret = !b1 >= b2;
+
+ ret = !getInt() == i1;
+ // expected-warning@-1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note@-2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note@-3 {{add parentheses around left hand side expression to silence this warning}}
+ // CHECK: to evaluate the comparison first
+ // CHECK: fix-it:"{{.*}}":{98:10-98:10}:"("
+ // CHECK: fix-it:"{{.*}}":{98:24-98:24}:")"
+ // CHECK: to silence this warning
+ // CHECK: fix-it:"{{.*}}":{98:9-98:9}:"("
+ // CHECK: fix-it:"{{.*}}":{98:18-98:18}:")"
+
+ ret = (!getInt()) == i1;
+ ret = !getBool() == b1;
+ return ret;
+}
OpenPOWER on IntegriCloud