Yes we seem to warn on the same things, strange that we didn't notice that.
Removed my warning and changed name.
//Anders
.......................................................................................................................
Anders Rönnholm Senior Engineer
Evidente ES East AB Warfvinges väg 34 SE-112 51 Stockholm Sweden
Mobile: +46 (0)70 912 42 54
E-mail: [email protected]
www.evidente.se
________________________________________
Från: Richard Trieu [[email protected]]
Skickat: den 27 mars 2014 04:28
Till: Anders Rönnholm
Cc: Richard Smith; [email protected]
Ämne: Re: [PATCH] check for Incorrect logic in operator
So, it looks like one of your new warning in compareBoolWithInt is sort of the
same as Per's patch to extend -Wtautological-compare warning for better bool
handling. Keep the existing logical, just remove the warning
warn_bool_and_int_comparison and the compareBoolWithInt() function.
warn-compint.c should also be removed.
+ if(BuildOpts.Observer)
+ BuildOpts.Observer->compareBoolWithInt(B);
Add space after 'if'
TautologicalCompareOperator isn't very descriptive. Once the bool compares are
removed, I think TautologicalOverlapCompare would be a better name for it.
On Fri, Mar 14, 2014 at 7:51 AM, Anders Rönnholm
<[email protected]<mailto:[email protected]>> wrote:
Hi Richard,
Modified the patch according to your comments.
Except for Use the APSInt ++operator as i don't want to increment L1 or L2.
//Anders
.......................................................................................................................
Anders Rönnholm Senior Engineer
Evidente ES East AB Warfvinges väg 34 SE-112 51 Stockholm Sweden
Mobile: +46 (0)70 912 42
54<tel:%2B46%20%280%2970%20912%2042%2054>
E-mail:
[email protected]<mailto:[email protected]>
www.evidente.se<http://www.evidente.se>
________________________________________
Från: Richard Trieu [[email protected]<mailto:[email protected]>]
Skickat: den 6 mars 2014 06:25
Till: Anders Rönnholm
Cc: Richard Smith; [email protected]<mailto:[email protected]>
Ämne: Re: [PATCH] check for Incorrect logic in operator
+ const IntegerLiteral *Literal1 =
+ dyn_cast<IntegerLiteral>(LHS->getRHS()->IgnoreParens());
Note that this won't work on negative values.
+ Literal1->EvaluateAsInt(L1,*Context);
+ Literal2->EvaluateAsInt(L2,*Context);
+
+ if (!L1 || !L2)
+ return TryResult();
if (!Literal1->EvaluateAsInt(L1, *Context) || Literal2->EvaluateAsInt(L2,
*Context))
return TryResult();
+ const llvm::APSInt MinValue =
+ llvm::APSInt::getMinValue(L1.getBitWidth(), L1.isUnsigned());
+ const llvm::APSInt MaxValue =
+ llvm::APSInt::getMaxValue(L1.getBitWidth(), L1.isUnsigned());
Move these into the array below.
+ llvm::APSInt I1 = llvm::APSInt(llvm::APInt(L1.getBitWidth(), 1),
+ L1.isUnsigned());
Don't need this.
+ // Value between Value1 and Value2
+ ((L1 < L2) ? L1 : L2) + I1,
Use the APSInt ++operator.
+ if(AlwaysTrue || AlwaysFalse) {
+ if(BuildOpts.Observer)
Add space between the if and the parentheses.
+ void compareAlwaysTrue(const BinaryOperator* B, bool isAlwaysTrue) {
BinaryOperator *B
+ if (LHS)
+ if (LHS->getLHS()->getExprLoc().isMacroID() ||
+ LHS->getRHS()->getExprLoc().isMacroID())
+ return;
+ if (RHS)
+ if (RHS->getLHS()->getExprLoc().isMacroID() ||
+ RHS->getRHS()->getExprLoc().isMacroID())
+ return;
if (!LHS || !RHS) first, then the two macro checks afterwards.
+ SourceRange DiagRange(B->getLHS()->getLocStart(),
B->getRHS()->getLocEnd());
+ S.Diag(B->getExprLoc(), diag::warn_operator_always_true_comparison)
+ << DiagRange << isAlwaysTrue;
Use B->getSourceRange() instead of creating a new SourceRange.
Same changes to compareBoolWithInt
- if (!isTemplateInstantiation)
+ if (!isTemplateInstantiation) {
+ LogicalErrorsHandler Leh(S);
+ AC.getCFGBuildOptions().Observer = &Leh;
CheckUnreachable(S, AC);
+ }
This forces your warning to be dependent on a different and unrelated function.
Instead, use:
if (Diags.getDiagnosticLevel(diag::warn_operator_always_true_comparison,
D->getLocStart()) {
LogicalErrorsHandler LEH(S);
AC.getCFGBuildOptions().Observer = &LEH;
AC.getCFG();
}
Place this before the "Emit delayed diagnostics." comment. CFG builds are
cached, so the observer must be attached to the first run of getCFG. It would
also be a good idea to have a reset observer function after just in case the
CFG is rebuilt later.
+def warn_operator_always_true_comparison : Warning<
+ "overlapping comparisons always evaluate to %select{false|true}0">,
+ InGroup<TautologicalCompareOperator>;
+def warn_bool_and_int_comparison : Warning<
+ "comparison of a boolean expression with an integer other than 0 or 1">,
+ InGroup<TautologicalCompareOperator>;
Add DefaultIgnore to these warnings.
def TautologicalOutOfRangeCompare :
DiagGroup<"tautological-constant-out-of-range-compare">;
def TautologicalCompare : DiagGroup<"tautological-compare",
[TautologicalOutOfRangeCompare]>;
+def TautologicalCompareOperator : DiagGroup<"tautological-compare-operator",
+ [TautologicalCompare]>;
TautologicalCompareOperator should be a subgroup of TautologicalCompare, not
the other way around
+// RUN: %clang_cc1 -fsyntax-only -verify -Wunreachable-code %s
Remove -Wunreachable-code and replace with -Wtautological-compare-operator in
both tests.
Add some cases with zero to warn-overlap.c
On Fri, Feb 21, 2014 at 5:48 AM, Anders Rönnholm
<[email protected]<mailto:[email protected]><mailto:[email protected]<mailto:[email protected]>>>
wrote:
Pinging this patch.
> -----Original Message-----
> From: Anders Rönnholm
> Sent: den 20 december 2013 13:35
> To: Richard Smith; Richard Trieu
> Cc:
> [email protected]<mailto:[email protected]><mailto:[email protected]<mailto:[email protected]>>
> Subject: Re: [PATCH] check for Incorrect logic in operator
>
> Tanks for your comments. New patch attached.
>
> -- We can have some warnings in a group on by default and some off by
> default, but this does suggest that we'd want a new warning subgroup for
> this warning.
> Do you want a new subgroup and how do you create one? I have moved it
> back to tautological and made a what i think is a subgroup.
You have made -Wtautogical-compare a subgroup of your warning. Switch it so
that using -Wtautological-compare will also turn on your warning.
>
> -- Instead of looking for an integer literal, see if the expression can be
> evaluated, and use that as the constant value. This will allow things like
> 1+2
> and -1.
> I will skip this because it will also generate false positives when it's x+1
> and
> can be evaluated.
What false positives are you seeing? I would think that negative numbers
should be checked with this warning as well.
>
> --Also, does your patch cause the unreachable-code warning to trigger in
> more cases now?
> I have tested it on llvm and the patch did not trigger more unreachable-code
> warnings.
>
> -- What is going on here?
> Comments in code, also slight refactoring Basicly it checks whether the
> expression is always true/false by checking following scenarios.
> x is less than the smallest literal.
> x is equal to the smallest literal.
> x is between smallest and largest literal.
> x is equal to the largest literal.
> x is greater than largest literal.
>
> For && both RHS and LHS needs to be true or false in every scenario for it to
> be regarded always true/false.
> For || either RHS and LHS needs to be true or false in every scenario for it
> to
> be regarded always true/false.
This refactoring makes the logic much clearer now.
>
> -- What's the plan with CFGCallBack? New methods added as needed for
> each new warning? Does the CFGBuilder need to support multiple
> CFGCallBack's at once?
> Yes new methods would have to be added. No i don't see a reason for the
> need of supporting multiple CFGCallBack's at once.
>
> //Anders
>
>
> .....................................................................................................................
> ..
> Anders Rönnholm Senior Engineer
> Evidente ES East AB Warfvinges väg 34 SE-112 51 Stockholm Sweden
>
> Mobile: +46 (0)70 912 42
> 54<tel:%2B46%20%280%2970%20912%2042%2054><tel:%2B46%20%280%2970%20912%2042%2054>
> E-mail:
> [email protected]<mailto:[email protected]><mailto:[email protected]<mailto:[email protected]>>
>
> www.evidente.se<http://www.evidente.se><http://www.evidente.se>
>
> ________________________________________
Index: lib/Sema/AnalysisBasedWarnings.cpp
===================================================================
--- lib/Sema/AnalysisBasedWarnings.cpp (revision 205417)
+++ lib/Sema/AnalysisBasedWarnings.cpp (working copy)
@@ -117,6 +117,34 @@
reachable_code::FindUnreachableCode(AC, S.getPreprocessor(), UC);
}
+/// \brief Warn on logical operator errors in CFGBuilder
+class LogicalErrorsHandler : public CFGCallback {
+ Sema &S;
+public:
+ LogicalErrorsHandler(Sema &S) : CFGCallback(),S(S) {}
+
+ bool HasMacroID(const Expr* E) {
+ // Recurse to children.
+ if (E->getExprLoc().isMacroID()) return true;
+
+ for (ConstStmtRange SubStmts = E->children(); SubStmts; ++SubStmts)
+ if (const Stmt *S = *SubStmts)
+ if (HasMacroID(cast<Expr>(S)))
+ return true;
+ return false;
+ }
+
+ void compareAlwaysTrue(const BinaryOperator* B, bool isAlwaysTrue) {
+ if (HasMacroID(B))
+ return;
+
+ SourceRange DiagRange = B->getSourceRange();
+ S.Diag(B->getExprLoc(), diag::warn_operator_always_true_comparison)
+ << DiagRange << isAlwaysTrue;
+ }
+};
+
+
//===----------------------------------------------------------------------===//
// Check for infinite self-recursion in functions
//===----------------------------------------------------------------------===//
@@ -1810,6 +1838,13 @@
.setAlwaysAdd(Stmt::AttributedStmtClass);
}
+ if (Diags.getDiagnosticLevel(diag::warn_operator_always_true_comparison,
+ D->getLocStart())) {
+ LogicalErrorsHandler LEH(S);
+ AC.getCFGBuildOptions().Observer = &LEH;
+ AC.getCFG();
+ AC.getCFGBuildOptions().Observer = 0;
+ }
// Emit delayed diagnostics.
if (!fscope->PossiblyUnreachableDiags.empty()) {
Index: lib/AST/Expr.cpp
===================================================================
--- lib/AST/Expr.cpp (revision 205417)
+++ lib/AST/Expr.cpp (working copy)
@@ -121,6 +121,8 @@
switch (UO->getOpcode()) {
case UO_Plus:
return UO->getSubExpr()->isKnownToHaveBooleanValue();
+ case UO_LNot:
+ return true;
default:
return false;
}
Index: lib/Analysis/CFG.cpp
===================================================================
--- lib/Analysis/CFG.cpp (revision 205417)
+++ lib/Analysis/CFG.cpp (working copy)
@@ -495,6 +495,208 @@
cfg->getBumpVectorContext());
}
+ /// \brief Find a relational comparison with an expression evaluating to a
+ /// boolean and a constant other than 0 and 1.
+ /// e.g. if ((x < y) == 10)
+ TryResult checkIncorrectRelationalOperator(const BinaryOperator *B) {
+ const IntegerLiteral *IntLiteral =
+ dyn_cast<IntegerLiteral>(B->getLHS()->IgnoreParens());
+ const Expr *BoolExpr = B->getRHS()->IgnoreParens();
+ bool IntFirst = true;
+ if (!IntLiteral) {
+ IntLiteral = dyn_cast<IntegerLiteral>(B->getRHS()->IgnoreParens());
+ BoolExpr = B->getLHS()->IgnoreParens();
+ IntFirst = false;
+ }
+
+ if (!IntLiteral || !BoolExpr->isKnownToHaveBooleanValue())
+ return TryResult();
+
+ llvm::APInt IntValue = IntLiteral->getValue();
+ if ((IntValue != 1) && (IntValue != 0)) {
+ bool IntLarger = !IntValue.isNegative();
+ BinaryOperatorKind Bok = B->getOpcode();
+ if (Bok == BO_GT || Bok == BO_GE) {
+ return TryResult(IntFirst != IntLarger);
+ } else {
+ return TryResult(IntFirst == IntLarger);
+ }
+ }
+ return TryResult();
+ }
+
+ /// Find a equality comparison with an expression evaluating to a boolean and
+ /// a constant other than 0 and 1.
+ /// e.g. if (!x == 10)
+ TryResult checkIncorrectEqualityOperator(const BinaryOperator *B) {
+ const IntegerLiteral *IntLiteral =
+ dyn_cast<IntegerLiteral>(B->getLHS()->IgnoreParens());
+ const Expr *BoolExpr = B->getRHS()->IgnoreParens();
+
+ if (!IntLiteral) {
+ IntLiteral = dyn_cast<IntegerLiteral>(B->getRHS()->IgnoreParens());
+ BoolExpr = B->getLHS()->IgnoreParens();
+ }
+
+ if (!IntLiteral || !BoolExpr->isKnownToHaveBooleanValue())
+ return TryResult();
+
+ llvm::APInt IntValue = IntLiteral->getValue();
+ if ((IntValue != 1) && (IntValue != 0)) {
+ return TryResult(B->getOpcode() != BO_EQ);
+ }
+ return TryResult();
+ }
+
+ TryResult analyzeLogicOperatorCondition(BinaryOperatorKind Relation,
+ const llvm::APSInt &Value1,
+ const llvm::APSInt &Value2) {
+ assert(Value1.isSigned() == Value2.isSigned());
+ switch (Relation) {
+ default:
+ return TryResult();
+ case BO_EQ:
+ return TryResult(Value1 == Value2);
+ case BO_NE:
+ return TryResult(Value1 != Value2);
+ case BO_LT:
+ return TryResult(Value1 < Value2);
+ case BO_LE:
+ return TryResult(Value1 <= Value2);
+ case BO_GT:
+ return TryResult(Value1 > Value2);
+ case BO_GE:
+ return TryResult(Value1 >= Value2);
+ }
+ }
+
+ /// \brief Find a pair of comparison expressions with or without parentheses
+ /// with a shared variable and constants and a logical operator between them
+ /// that always evaluates to either true or false.
+ /// e.g. if (x != 3 || x != 4)
+ TryResult checkIncorrectLogicOperator(const BinaryOperator *B) {
+ const BinaryOperator *LHS =
+ dyn_cast<BinaryOperator>(B->getLHS()->IgnoreParens());
+ const BinaryOperator *RHS =
+ dyn_cast<BinaryOperator>(B->getRHS()->IgnoreParens());
+ if (!LHS || !RHS)
+ return TryResult();
+
+ if (!LHS->isComparisonOp() || !RHS->isComparisonOp())
+ return TryResult();
+
+ BinaryOperatorKind BO1 = LHS->getOpcode();
+ const DeclRefExpr *Decl1 =
+ dyn_cast<DeclRefExpr>(LHS->getLHS()->IgnoreParenImpCasts());
+ const IntegerLiteral *Literal1 =
+ dyn_cast<IntegerLiteral>(LHS->getRHS()->IgnoreParens());
+ if (!Decl1 && !Literal1) {
+ if (BO1 == BO_GT)
+ BO1 = BO_LT;
+ else if (BO1 == BO_GE)
+ BO1 = BO_LE;
+ else if (BO1 == BO_LT)
+ BO1 = BO_GT;
+ else if (BO1 == BO_LE)
+ BO1 = BO_GE;
+ Decl1 =
+ dyn_cast<DeclRefExpr>(LHS->getRHS()->IgnoreParenImpCasts());
+ Literal1 =
+ dyn_cast<IntegerLiteral>(LHS->getLHS()->IgnoreParens());
+ }
+
+ if (!Decl1 || !Literal1)
+ return TryResult();
+
+ BinaryOperatorKind BO2 = RHS->getOpcode();
+ const DeclRefExpr *Decl2 =
+ dyn_cast<DeclRefExpr>(RHS->getLHS()->IgnoreParenImpCasts());
+ const IntegerLiteral *Literal2 =
+ dyn_cast<IntegerLiteral>(RHS->getRHS()->IgnoreParens());
+ if (!Decl2 && !Literal2) {
+ if (BO2 == BO_GT)
+ BO2 = BO_LT;
+ else if (BO2 == BO_GE)
+ BO2 = BO_LE;
+ else if (BO2 == BO_LT)
+ BO2 = BO_GT;
+ else if (BO2 == BO_LE)
+ BO2 = BO_GE;
+ Decl2 =
+ dyn_cast<DeclRefExpr>(RHS->getRHS()->IgnoreParenImpCasts());
+ Literal2 =
+ dyn_cast<IntegerLiteral>(RHS->getLHS()->IgnoreParens());
+ }
+
+ if (!Decl2 || !Literal2)
+ return TryResult();
+
+ // Check that it is the same variable on both sides.
+ if (Decl1->getDecl() != Decl2->getDecl())
+ return TryResult();
+
+ llvm::APSInt L1, L2;
+
+ if (!Literal1->EvaluateAsInt(L1,*Context) ||
+ !Literal2->EvaluateAsInt(L2,*Context))
+ return TryResult();
+
+ // Can't compare signed with unsigned or with different bit width.
+ if (L1.isSigned() != L2.isSigned() ||
+ L1.getBitWidth() != L2.getBitWidth())
+ return TryResult();
+
+ // Values that will be used to determine if result of logical
+ // operator is always true/false
+ const llvm::APSInt Values[] = {
+ // Value less than both Value1 and Value2
+ llvm::APSInt::getMinValue(L1.getBitWidth(), L1.isUnsigned()),
+ // L1
+ L1,
+ // Value between Value1 and Value2
+ ((L1 < L2) ? L1 : L2) + llvm::APSInt(llvm::APInt(L1.getBitWidth(), 1),
+ L1.isUnsigned()),
+ // L2
+ L2,
+ // Value greater than both Value1 and Value2
+ llvm::APSInt::getMaxValue(L1.getBitWidth(), L1.isUnsigned()),
+ };
+
+ // Check whether expression is always true/false by evaluating the following
+ // * variable x is less than the smallest literal.
+ // * variable x is equal to the smallest literal.
+ // * Variable x is between smallest and largest literal.
+ // * Variable x is equal to the largest literal.
+ // * Variable x is greater than largest literal.
+ bool AlwaysTrue = true, AlwaysFalse = true;
+ for (unsigned int ValueIndex = 0;
+ ValueIndex < sizeof(Values) / sizeof(Values[0]);
+ ++ValueIndex) {
+ llvm::APSInt Value = Values[ValueIndex];
+ TryResult Res1, Res2;
+ Res1 = analyzeLogicOperatorCondition(BO1, Value, L1);
+ Res2 = analyzeLogicOperatorCondition(BO2, Value, L2);
+
+ if (!Res1.isKnown() || !Res2.isKnown())
+ return TryResult();
+
+ if (B->getOpcode() == BO_LAnd) {
+ AlwaysTrue &= (Res1.isTrue() && Res2.isTrue());
+ AlwaysFalse &= !(Res1.isTrue() && Res2.isTrue());
+ } else {
+ AlwaysTrue &= (Res1.isTrue() || Res2.isTrue());
+ AlwaysFalse &= !(Res1.isTrue() || Res2.isTrue());
+ }
+ }
+
+ if (AlwaysTrue || AlwaysFalse) {
+ if (BuildOpts.Observer)
+ BuildOpts.Observer->compareAlwaysTrue(B, AlwaysTrue);
+ return TryResult(AlwaysTrue);
+ }
+ return TryResult();
+ }
+
/// Try and evaluate an expression to an integer constant.
bool tryEvaluate(Expr *S, Expr::EvalResult &outResult) {
if (!BuildOpts.PruneTriviallyFalseEdges)
@@ -577,10 +779,22 @@
// is determined by the RHS: X && 0 -> 0, X || 1 -> 1.
if (RHS.isTrue() == (Bop->getOpcode() == BO_LOr))
return RHS.isTrue();
+ } else {
+ TryResult BopRes = checkIncorrectLogicOperator(Bop);
+ if (BopRes.isKnown())
+ return BopRes.isTrue();
}
}
return TryResult();
+ } else if (Bop->isEqualityOp()) {
+ TryResult BopRes = checkIncorrectEqualityOperator(Bop);
+ if (BopRes.isKnown())
+ return BopRes.isTrue();
+ } else if (Bop->isRelationalOp()) {
+ TryResult BopRes = checkIncorrectRelationalOperator(Bop);
+ if (BopRes.isKnown())
+ return BopRes.isTrue();
}
}
@@ -1320,6 +1534,8 @@
else {
RHSBlock->setTerminator(Term);
TryResult KnownVal = tryEvaluateBool(RHS);
+ if (!KnownVal.isKnown())
+ KnownVal = tryEvaluateBool(B);
addSuccessor(RHSBlock, TrueBlock, !KnownVal.isFalse());
addSuccessor(RHSBlock, FalseBlock, !KnownVal.isTrue());
}
Index: test/Sema/warn-overlap.c
===================================================================
--- test/Sema/warn-overlap.c (revision 0)
+++ test/Sema/warn-overlap.c (revision 0)
@@ -0,0 +1,58 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wtautological-overlap-compare %s
+
+#define mydefine 2
+
+void f(int x) {
+ int y = 0;
+
+ // > || <
+ if (x > 2 || x < 1) { }
+ if (x > 2 || x < 2) { }
+ if (x != 2 || x != 3) { } // expected-warning {{overlapping comparisons always evaluate to true}}
+ if (x > 2 || x < 3) { } // expected-warning {{overlapping comparisons always evaluate to true}}
+ if (x > 0 || x < 0) { }
+
+ if (x > 2 || x <= 1) { }
+ if (x > 2 || x <= 2) { } // expected-warning {{overlapping comparisons always evaluate to true}}
+ if (x > 2 || x <= 3) { } // expected-warning {{overlapping comparisons always evaluate to true}}
+
+ if (x >= 2 || x < 1) { }
+ if (x >= 2 || x < 2) { } // expected-warning {{overlapping comparisons always evaluate to true}}
+ if (x >= 2 || x < 3) { } // expected-warning {{overlapping comparisons always evaluate to true}}
+
+ if (x >= 2 || x <= 1) { } // expected-warning {{overlapping comparisons always evaluate to true}}
+ if (x >= 2 || x <= 2) { } // expected-warning {{overlapping comparisons always evaluate to true}}
+ if (x >= 2 || x <= 3) { } // expected-warning {{overlapping comparisons always evaluate to true}}
+ if (x >= 0 || x <= 0) { } // expected-warning {{overlapping comparisons always evaluate to true}}
+
+ // > && <
+ if (x > 2 && x < 1) { } // expected-warning {{overlapping comparisons always evaluate to false}}
+ if (x > 2 && x < 2) { } // expected-warning {{overlapping comparisons always evaluate to false}}
+ if (x > 2 && x < 3) { } // expected-warning {{overlapping comparisons always evaluate to false}}
+ if (x > 0 && x < 1) { } // expected-warning {{overlapping comparisons always evaluate to false}}
+
+ if (x > 2 && x <= 1) { } // expected-warning {{overlapping comparisons always evaluate to false}}
+ if (x > 2 && x <= 2) { } // expected-warning {{overlapping comparisons always evaluate to false}}
+ if (x > 2 && x <= 3) { }
+
+ if (x >= 2 && x < 1) { } // expected-warning {{overlapping comparisons always evaluate to false}}
+ if (x >= 2 && x < 2) { } // expected-warning {{overlapping comparisons always evaluate to false}}
+ if (x >= 2 && x < 3) { }
+ if (x >= 0 && x < 0) { } // expected-warning {{overlapping comparisons always evaluate to false}}
+
+ if (x >= 2 && x <= 1) { } // expected-warning {{overlapping comparisons always evaluate to false}}
+ if (x >= 2 && x <= 2) { }
+ if (x >= 2 && x <= 3) { }
+
+ // !=, ==, ..
+ if (x != 2 || x != 3) { } // expected-warning {{overlapping comparisons always evaluate to true}}
+ if (x != 2 || x < 3) { } // expected-warning {{overlapping comparisons always evaluate to true}}
+ if (x == 2 && x == 3) { } // expected-warning {{overlapping comparisons always evaluate to false}}
+ if (x == 2 && x > 3) { } // expected-warning {{overlapping comparisons always evaluate to false}}
+ if (x == 3 && x < 0) { } // expected-warning {{overlapping comparisons always evaluate to false}}
+ if (3 == x && x < 0) { } // expected-warning {{overlapping comparisons always evaluate to false}}
+
+ if (x == mydefine && x > 3) { }
+ if (x == (mydefine + 1) && x > 3) { }
+}
+
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td (revision 205417)
+++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)
@@ -6379,6 +6379,9 @@
def warn_comparison_always : Warning<
"%select{self-|array }0comparison always evaluates to %select{false|true|a constant}1">,
InGroup<TautologicalCompare>;
+def warn_operator_always_true_comparison : Warning<
+ "overlapping comparisons always evaluate to %select{false|true}0">,
+ InGroup<TautologicalOverlapCompare>, DefaultIgnore;
def warn_stringcompare : Warning<
"result of comparison against %select{a string literal|@encode}0 is "
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td (revision 205417)
+++ include/clang/Basic/DiagnosticGroups.td (working copy)
@@ -290,9 +290,11 @@
def StrncatSize : DiagGroup<"strncat-size">;
def TautologicalOutOfRangeCompare : DiagGroup<"tautological-constant-out-of-range-compare">;
def TautologicalPointerCompare : DiagGroup<"tautological-pointer-compare">;
+def TautologicalOverlapCompare : DiagGroup<"tautological-overlap-compare">;
def TautologicalCompare : DiagGroup<"tautological-compare",
[TautologicalOutOfRangeCompare,
- TautologicalPointerCompare]>;
+ TautologicalPointerCompare,
+ TautologicalOverlapCompare]>;
def HeaderHygiene : DiagGroup<"header-hygiene">;
def DuplicateDeclSpecifier : DiagGroup<"duplicate-decl-specifier">;
def CompareDistinctPointerType : DiagGroup<"compare-distinct-pointer-types">;
Index: include/clang/Analysis/CFG.h
===================================================================
--- include/clang/Analysis/CFG.h (revision 205417)
+++ include/clang/Analysis/CFG.h (working copy)
@@ -47,6 +47,7 @@
class CXXRecordDecl;
class CXXDeleteExpr;
class CXXNewExpr;
+ class BinaryOperator;
/// CFGElement - Represents a top-level expression in a basic block.
class CFGElement {
@@ -698,6 +699,15 @@
}
};
+/// \brief CFGCallback defines methods that should be called when a logical
+/// operator error is found when building the CFG.
+class CFGCallback {
+public:
+ CFGCallback(){}
+ virtual void compareAlwaysTrue(const BinaryOperator *B, bool isAlwaysTrue) {};
+ virtual ~CFGCallback(){}
+};
+
/// CFG - Represents a source-level, intra-procedural CFG that represents the
/// control-flow of a Stmt. The Stmt can represent an entire function body,
/// or a single expression. A CFG will always contain one empty block that
@@ -716,7 +726,7 @@
public:
typedef llvm::DenseMap<const Stmt *, const CFGBlock*> ForcedBlkExprs;
ForcedBlkExprs **forcedBlkExprs;
-
+ CFGCallback* Observer;
bool PruneTriviallyFalseEdges;
bool AddEHEdges;
bool AddInitializers;
@@ -740,7 +750,7 @@
}
BuildOptions()
- : forcedBlkExprs(0), PruneTriviallyFalseEdges(true)
+ : forcedBlkExprs(0), Observer(0), PruneTriviallyFalseEdges(true)
,AddEHEdges(false)
,AddInitializers(false)
,AddImplicitDtors(false)
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
