[PATCH] New warning to detect when a logical not is applied to the LHS of a comparison when the intent was for the logical not to apply to the whole comparison
Richard Trieu
rtrieu at google.com
Thu May 30 20:27:27 PDT 2013
Update patch to latest revision. Moved warning to be part of -Wparenthesis.
Hi rsmith,
http://llvm-reviews.chandlerc.com/D505
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D505?vs=1514&id=2203#toc
Files:
lib/Sema/SemaExpr.cpp
test/SemaCXX/warn-logical-not-compare.cpp
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -7238,6 +7238,45 @@
}
}
+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,
@@ -7258,6 +7297,7 @@
Expr *RHSStripped = RHS.get()->IgnoreParenImpCasts();
checkEnumComparison(*this, Loc, LHS.get(), RHS.get());
+ diagnoseLogicalNotOnLHSofComparison(*this, LHS, RHS, Loc, OpaqueOpc);
if (!LHSType->hasFloatingRepresentation() &&
!(LHSType->isBlockPointerType() && IsRelational) &&
Index: test/SemaCXX/warn-logical-not-compare.cpp
===================================================================
--- test/SemaCXX/warn-logical-not-compare.cpp
+++ test/SemaCXX/warn-logical-not-compare.cpp
@@ -0,0 +1,69 @@
+// RUN: %clang_cc1 -fsyntax-only -Wlogical-not-parentheses -verify %s
+
+bool getBool();
+int getInt();
+
+bool test1(int i1, int i2, bool b1, bool b2) {
+ bool ret;
+
+ ret = !i1 == i2;
+ // expected-warning at -1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note at -2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note at -3 {{add parentheses around left hand side expression to silence this warning}}
+
+ ret = !i1 != i2;
+ //expected-warning at -1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note at -2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note at -3 {{add parentheses around left hand side expression to silence this warning}}
+
+ ret = !i1 < i2;
+ //expected-warning at -1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note at -2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note at -3 {{add parentheses around left hand side expression to silence this warning}}
+
+ ret = !i1 > i2;
+ //expected-warning at -1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note at -2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note at -3 {{add parentheses around left hand side expression to silence this warning}}
+
+ ret = !i1 <= i2;
+ //expected-warning at -1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note at -2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note at -3 {{add parentheses around left hand side expression to silence this warning}}
+
+ ret = !i1 >= i2;
+ //expected-warning at -1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note at -2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note at -3 {{add parentheses around left hand side expression to silence this warning}}
+
+ 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 at -1 {{logical not is only applied to the left hand side of this comparison}}
+ // expected-note at -2 {{add parentheses after the '!' to evaluate the comparison first}}
+ // expected-note at -3 {{add parentheses around left hand side expression to silence this warning}}
+
+ ret = (!getInt()) == i1;
+ ret = !getBool() == b1;
+ return ret;
+}
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -136,6 +136,7 @@
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 ParenthesesOnEquality : DiagGroup<"parentheses-equality">;
def Parentheses : DiagGroup<"parentheses",
[LogicalOpParentheses,
+ LogicalNotParentheses,
BitwiseOpParentheses,
ShiftOpParentheses,
OverloadedShiftOpParentheses,
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -4435,6 +4435,14 @@
"%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<
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D505.4.patch
Type: text/x-patch
Size: 7361 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130530/e0cce30b/attachment.bin>
More information about the cfe-commits
mailing list