[PATCH] check for Incorrect logic in operator
Anders Rönnholm
Anders.Ronnholm at evidente.se
Fri Feb 21 05:48:18 PST 2014
Pinging this patch.
> -----Original Message-----
> From: Anders Rönnholm
> Sent: den 20 december 2013 13:35
> To: Richard Smith; Richard Trieu
> Cc: cfe-commits at cs.uiuc.edu
> 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.
>
> -- 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.
>
> --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.
>
> -- 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
> E-mail: Anders.Ronnholm at evidente.se
>
> www.evidente.se
>
> ________________________________________
> Från: metafoo at gmail.com [metafoo at gmail.com] för Richard Smith
> [richard at metafoo.co.uk]
> Skickat: den 13 december 2013 19:18
> Till: Richard Trieu
> Cc: Anders Rönnholm; cfe-commits at cs.uiuc.edu
> Ämne: Re: [PATCH] check for Incorrect logic in operator
>
> On Thu, Dec 12, 2013 at 7:02 PM, Richard Trieu
> <rtrieu at google.com<mailto:rtrieu at google.com>> wrote:
>
>
>
> On Wed, Dec 11, 2013 at 3:59 AM, Anders Rönnholm
> <Anders.Ronnholm at evidente.se<mailto:Anders.Ronnholm at evidente.se>>
> wrote:
> Hi,
>
> I haven't received any comments on this patch. Is anyone reviewing it?
>
> //Anders
>
> From: Anders Rönnholm
> Sent: den 21 november 2013 08:52
> To: 'Anna Zaks'; Richard Trieu
> Cc: Jordan Rose; Ted Kremenek; cfe-commits at cs.uiuc.edu<mailto:cfe-
> commits at cs.uiuc.edu>
> Subject: RE: [PATCH] check for Incorrect logic in operator
>
> Hi,
>
> Here is an updated patch I'd like to get reviewed. The warnings have been
> moved to the unreachable-code group, I hope that's fine.
>
> Thanks,
> Anders
>
>
> I don't like the warnings in the unreachable-code group, since in the always
> true case, it is the opposite of unreachable code.
>
> The tautological comparison group makes more sense for this warning.
>
> However, existing the existing warning groups that this would belong to are
> on by default, which typically exclude CFG warnings.
>
> 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.
>
> We may need a temporary solution until we merge the old and new
> warnings.
>
> Also, does your patch cause the unreachable-code warning to trigger in more
> cases now?
>
> More comments inline.
>
> Index: lib/Analysis/CFG.cpp
> ==========================================================
> =========
> --- lib/Analysis/CFG.cpp (revision 194562)
> +++ lib/Analysis/CFG.cpp (working copy)
> @@ -483,6 +483,192 @@
> B->addSuccessor(S, 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)) {
> + BuildOpts.Observer->compareBoolWithInt(B);
>
> Check that BuildOpts.Observer is not null before dereferencing.
>
> + 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)) {
> + BuildOpts.Observer->compareBoolWithInt(B);
> + return TryResult(B->getOpcode() != BO_EQ);
> + }
> + return TryResult();
> + }
> +
> + bool analyzeLogicOperatorCondition(BinaryOperatorKind Relation,
> + const llvm::APSInt Value1,
> + const llvm::APSInt Value2) {
>
> A switch statement would be nicer here. Also, add an assert that the two
> values have the same signedness and bit width.
>
> + return (Relation == BO_EQ && (Value1 == Value2)) ||
> + (Relation == BO_NE && (Value1 != Value2)) ||
> + (Relation == BO_LT && (Value1 < Value2)) ||
> + (Relation == BO_LE && (Value1 <= Value2)) ||
> + (Relation == BO_GT && (Value1 > Value2)) ||
> + (Relation == BO_GE && (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();
> +
>
> 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.
>
> + 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()->IgnoreImpCasts()-
> >IgnoreParens())
> + ;
>
> ->IgnoreParenImpCasts() instead of ->IgnoreImpCasts()->IgnoreParens()
>
> + 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()->IgnoreImpCasts()-
> >IgnoreParens());
> + Literal2 = dyn_cast<IntegerLiteral>(RHS->getLHS()->IgnoreParens());
> + }
> +
> + if (!Decl2 || !Literal2)
> + return TryResult();
> +
> + // Check that it is the same variable on both sides.
> + if (Decl1->getNameInfo().getAsString() !=
> + Decl2->getNameInfo().getAsString())
>
> Decl1->getDecl() != Decl2->getDecl()
>
> + return TryResult();
> +
> + // evaluate if expression is always true/false
> + llvm::APSInt L1,L2;
> + if (!Literal1->EvaluateAsInt(L1,*Context) ||
> + !Literal2->EvaluateAsInt(L2,*Context))
> + return TryResult();
> +
> + const llvm::APSInt MinValueL1 =
> + llvm::APSInt::getMinValue(L1.getBitWidth(),L1.isUnsigned());
> + const llvm::APSInt MaxValueL1 =
> + llvm::APSInt::getMaxValue(L1.getBitWidth(),L1.isUnsigned());
> +
> + const llvm::APSInt MinValueL2 =
> + llvm::APSInt::getMinValue(L2.getBitWidth(),L2.isUnsigned());
> + const llvm::APSInt MaxValueL2 =
> + llvm::APSInt::getMaxValue(L2.getBitWidth(),L2.isUnsigned());
> + bool AlwaysTrue = true, AlwaysFalse = true;
>
> What is going on here?
> + for (int Offset = -3; Offset <=3; ++Offset) {
> + for (int Selvalue = 1; Selvalue <= 2; Selvalue++) {
> +
> + llvm::APSInt SelInt = (Selvalue==1 ? L1 : L2);
> + llvm::APSInt SelMin = (Selvalue==1 ? MinValueL1 : MinValueL2);
> + llvm::APSInt SelMax = (Selvalue==1 ? MaxValueL1 : MaxValueL2);
> +
> + llvm::APSInt OffsetAPS =
> + llvm::APSInt(llvm::APInt(SelInt.getBitWidth(), Offset),
> + SelInt.isUnsigned());
> +
> + if ((SelInt + OffsetAPS) > SelMax)
> + continue;
> + else if ((SelInt + OffsetAPS) < SelMin)
> + continue;
> + else
> + SelInt += OffsetAPS;
> +
> + bool Res1,Res2;
> + Res1 = analyzeLogicOperatorCondition(BO1,SelInt, L1);
> + Res2 = analyzeLogicOperatorCondition(BO2,SelInt, L2);
> + if (B->getOpcode() == BO_LAnd) {
> + AlwaysTrue &= (Res1 && Res2);
> + AlwaysFalse &= !(Res1 && Res2);
> + } else {
> + AlwaysTrue &= (Res1 || Res2);
> + AlwaysFalse &= !(Res1 || Res2);
> + }
> + }
> + }
> +
> + if(AlwaysTrue || AlwaysFalse) {
> + 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)
> @@ -565,13 +751,24 @@
> // 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();
> }
> }
> -
> bool Result;
> if (E->EvaluateAsBooleanCondition(Result, *Context))
> return Result;
> @@ -1311,6 +1508,8 @@
> else {
> RHSBlock->setTerminator(Term);
> TryResult KnownVal = tryEvaluateBool(RHS);
> + if (!KnownVal.isKnown())
> + KnownVal = tryEvaluateBool(B);
> addSuccessor(RHSBlock, KnownVal.isFalse() ? NULL : TrueBlock);
> addSuccessor(RHSBlock, KnownVal.isTrue() ? NULL : FalseBlock);
> }
> Index: lib/Sema/AnalysisBasedWarnings.cpp
> ==========================================================
> =========
> --- lib/Sema/AnalysisBasedWarnings.cpp (revision 194562)
> +++ lib/Sema/AnalysisBasedWarnings.cpp (working copy)
> @@ -77,6 +77,60 @@
> reachable_code::FindUnreachableCode(AC, UC); }
>
> +/// \brief Warn on logical operator errors in CFGBuilder class
> +LogicalErrorsHandler : public CFGCallback {
> + Sema &S;
> +public:
> + LogicalErrorsHandler(Sema &S) : CFGCallback(),S(S) {}
> + void compareAlwaysTrue(const BinaryOperator* B, bool isAlwaysTrue) {
> + if (B->getLHS()->getExprLoc().isMacroID() ||
> + B->getRHS()->getExprLoc().isMacroID())
> + return;
> +
> + const BinaryOperator *LHS =
> + dyn_cast<BinaryOperator>(B->getLHS()->IgnoreParens());
> + const BinaryOperator *RHS =
> + dyn_cast<BinaryOperator>(B->getRHS()->IgnoreParens());
> +
> + if (LHS)
> + if (LHS->getLHS()->getExprLoc().isMacroID() ||
> + LHS->getRHS()->getExprLoc().isMacroID())
> + return;
> + if (RHS)
> + if (RHS->getLHS()->getExprLoc().isMacroID() ||
> + RHS->getRHS()->getExprLoc().isMacroID())
> + return;
> +
> + SourceRange DiagRange(B->getLHS()->getLocStart(), B->getRHS()-
> >getLocEnd());
> + S.Diag(B->getExprLoc(), diag::warn_operator_always_true_comparison)
> + << DiagRange << isAlwaysTrue;
> + }
> +
> + void compareBoolWithInt(const BinaryOperator* B) {
> + if (B->getLHS()->getExprLoc().isMacroID() ||
> + B->getRHS()->getExprLoc().isMacroID())
> + return;
> +
> + const BinaryOperator *LHS =
> + dyn_cast<BinaryOperator>(B->getLHS()->IgnoreParens());
> + const BinaryOperator *RHS =
> + dyn_cast<BinaryOperator>(B->getRHS()->IgnoreParens());
> +
> + if (LHS)
> + if (LHS->getLHS()->getExprLoc().isMacroID() ||
> + LHS->getRHS()->getExprLoc().isMacroID())
> + return;
> + if (RHS)
> + if (RHS->getLHS()->getExprLoc().isMacroID() ||
> + RHS->getRHS()->getExprLoc().isMacroID())
> + return;
> +
> + SourceRange DiagRange(B->getLHS()->getLocStart(), B->getRHS()-
> >getLocEnd());
> + S.Diag(B->getExprLoc(), diag::warn_bool_and_int_comparison) <<
> +DiagRange;
> + }
> +};
> +
> +
> //===----------------------------------------------------------------------===//
> // Check for missing return value.
> //===----------------------------------------------------------------------===//
> @@ -1723,8 +1777,11 @@
> bool isTemplateInstantiation = false;
> if (const FunctionDecl *Function = dyn_cast<FunctionDecl>(D))
> isTemplateInstantiation = Function->isTemplateInstantiation();
> - if (!isTemplateInstantiation)
> + if (!isTemplateInstantiation) {
> + LogicalErrorsHandler Leh(S);
> + AC.getCFGBuildOptions().Observer = &Leh;
> CheckUnreachable(S, AC);
> + }
> }
>
> // Check for thread safety violations
> Index: lib/AST/Expr.cpp
> ==========================================================
> =========
> --- lib/AST/Expr.cpp (revision 194562)
> +++ lib/AST/Expr.cpp (working copy)
> @@ -152,6 +152,8 @@
> switch (UO->getOpcode()) {
> case UO_Plus:
> return UO->getSubExpr()->isKnownToHaveBooleanValue();
> + case UO_LNot:
> + return true;
> default:
> return false;
> }
> Index: test/Sema/warn-overlap.c
> ==========================================================
> =========
> --- test/Sema/warn-overlap.c (revision 0)
> +++ test/Sema/warn-overlap.c (working copy)
>
> Add some tests with different variable types and different constant types.
>
> @@ -0,0 +1,51 @@
> +// RUN: %clang_cc1 -fsyntax-only -verify -Wunreachable-code %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 {{logical disjunction
> +always evaluates to true}}
> + if (x > 2 || x < 3) { } // expected-warning {{logical disjunction
> +always evaluates to true}}
> +
> + if (x > 2 || x <= 1) { }
> + if (x > 2 || x <= 2) { } // expected-warning {{logical disjunction
> + always evaluates to true}} if (x > 2 || x <= 3) { } //
> + expected-warning {{logical disjunction always evaluates to true}}
> +
> + if (x >= 2 || x < 1) { }
> + if (x >= 2 || x < 2) { } // expected-warning {{logical disjunction
> + always evaluates to true}} if (x >= 2 || x < 3) { } //
> + expected-warning {{logical disjunction always evaluates to true}}
> +
> + if (x >= 2 || x <= 1) { } // expected-warning {{logical disjunction
> + always evaluates to true}} if (x >= 2 || x <= 2) { } //
> + expected-warning {{logical disjunction always evaluates to true}} if
> + (x >= 2 || x <= 3) { } // expected-warning {{logical disjunction
> + always evaluates to true}}
> +
> + // > && <
> + if (x > 2 && x < 1) { } // expected-warning {{logical disjunction
> + always evaluates to false}} if (x > 2 && x < 2) { } //
> + expected-warning {{logical disjunction always evaluates to false}} if
> + (x > 2 && x < 3) { } // expected-warning {{logical disjunction always
> + evaluates to false}}
> +
> + if (x > 2 && x <= 1) { } // expected-warning {{logical disjunction
> + always evaluates to false}} if (x > 2 && x <= 2) { } //
> + expected-warning {{logical disjunction always evaluates to false}} if
> + (x > 2 && x <= 3) { }
> +
> + if (x >= 2 && x < 1) { } // expected-warning {{logical disjunction
> + always evaluates to false}} if (x >= 2 && x < 2) { } //
> + expected-warning {{logical disjunction always evaluates to false}} if
> + (x >= 2 && x < 3) { }
> +
> + if (x >= 2 && x <= 1) { } // expected-warning {{logical disjunction
> + always evaluates to false}} if (x >= 2 && x <= 2) { } if (x >= 2 &&
> + x <= 3) { }
> +
> + // !=, ==, ..
> + if (x != 2 || x != 3) { } // expected-warning {{logical disjunction always
> evaluates to true}}
> + if (x != 2 || x < 3) { } // expected-warning {{logical disjunction always
> evaluates to true}}
> + if (x == 2 && x == 3) { } // expected-warning {{logical disjunction always
> evaluates to false}}
> + if (x == 2 && x > 3) { } // expected-warning {{logical disjunction always
> evaluates to false}}
> +
> + if (x == mydefine && x > 3) { } // no-warning
> + if (x == 2 && y > 3) { } // no-warning }
> +
> Index: test/Sema/warn-compint.c
> ==========================================================
> =========
> --- test/Sema/warn-compint.c (revision 0)
> +++ test/Sema/warn-compint.c (working copy)
> @@ -0,0 +1,39 @@
> +// RUN: %clang_cc1 -fsyntax-only -verify -Wunreachable-code %s
> +
> +#define mydefine 2
> +
> +void err(int x, int y) {
> +
> + if (!x == 10) {} // expected-warning {{comparison of a boolean expression
> with an integer other than 0 or 1}}
> + if (!x != 10) {} // expected-warning {{comparison of a boolean expression
> with an integer other than 0 or 1}}
> + if (!x < 10) {} // expected-warning {{comparison of a boolean expression
> with an integer other than 0 or 1}}
> + if (!x > 10) {} // expected-warning {{comparison of a boolean expression
> with an integer other than 0 or 1}}
> + if (!x >= 10) {} // expected-warning {{comparison of a boolean expression
> with an integer other than 0 or 1}}
> + if (!x <= 10) {} // expected-warning {{comparison of a boolean
> + expression with an integer other than 0 or 1}}
> +
> + if ((x < y) == 10) {} // expected-warning {{comparison of a boolean
> expression with an integer other than 0 or 1}}
> + if ((x < y) != 10) {} // expected-warning {{comparison of a boolean
> expression with an integer other than 0 or 1}}
> + if ((x < y) < 10) {} // expected-warning {{comparison of a boolean
> expression with an integer other than 0 or 1}}
> + if ((x < y) > 10) {} // expected-warning {{comparison of a boolean
> expression with an integer other than 0 or 1}}
> + if ((x < y) >= 10) {} // expected-warning {{comparison of a boolean
> expression with an integer other than 0 or 1}}
> + if ((x < y) <= 10) {} // expected-warning {{comparison of a
> +boolean expression with an integer other than 0 or 1}} }
> +
> +void noerr(int x, int y) {
> +
> + if (!x == 1) {}
> + if (!x != 0) {}
> + if (!x < 1) {}
> + if (!x > 0) {}
> + if (!x >= 1) {}
> + if (!x <= 0) {}
> +
> + if ((x < y) == 1) {}
> + if ((x < y) != 0) {}
> + if ((x < y) < 1) {}
> + if ((x < y) > 0) {}
> + if ((x < y) >= 1) {}
> + if ((x < y) <= 0) {}
> +
> + if ((x < mydefine) <= 10) {}
> +} // no-warning
> Index: include/clang/Basic/DiagnosticSemaKinds.td
> ==========================================================
> =========
> --- include/clang/Basic/DiagnosticSemaKinds.td (revision 194562)
> +++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)
> @@ -339,6 +339,12 @@
> InGroup<MissingNoreturn>, DefaultIgnore; def warn_unreachable :
> Warning<"will never be executed">,
> InGroup<DiagGroup<"unreachable-code">>, DefaultIgnore;
>
> Still don't like the logical disjunction text. I think it would be confusing for
> users.
>
> +def warn_operator_always_true_comparison : Warning<
> + "logical disjunction always evaluates to %select{false|true}0">,
> + InGroup<DiagGroup<"unreachable-code">>, DefaultIgnore; def
> +warn_bool_and_int_comparison : Warning<
> + "comparison of a boolean expression with an integer other than 0 or
> +1">,
> + InGroup<DiagGroup<"unreachable-code">>, DefaultIgnore;
>
> /// Built-in functions.
> def ext_implicit_lib_function_decl : ExtWarn<
> Index: include/clang/Analysis/CFG.h
> ==========================================================
> =========
> --- include/clang/Analysis/CFG.h (revision 194562)
> +++ include/clang/Analysis/CFG.h (working copy)
> @@ -45,6 +45,7 @@
> class ASTContext;
> class CXXRecordDecl;
> class CXXDeleteExpr;
> + class BinaryOperator;
>
> /// CFGElement - Represents a top-level expression in a basic block.
> class CFGElement {
> @@ -609,6 +610,16 @@
> }
> };
>
> 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?
>
> +/// \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 void compareBoolWithInt(const BinaryOperator *B) {};
> + 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 @@ -627,7 +638,7 @@
> public:
> typedef llvm::DenseMap<const Stmt *, const CFGBlock*> ForcedBlkExprs;
> ForcedBlkExprs **forcedBlkExprs;
> -
> + CFGCallback* Observer;
> bool PruneTriviallyFalseEdges;
> bool AddEHEdges;
> bool AddInitializers;
> @@ -650,7 +661,7 @@
> }
>
> BuildOptions()
> - : forcedBlkExprs(0), PruneTriviallyFalseEdges(true)
> + : forcedBlkExprs(0), Observer(0), PruneTriviallyFalseEdges(true)
> ,AddEHEdges(false)
> ,AddInitializers(false)
> ,AddImplicitDtors(false)
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu<mailto:cfe-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: incorrectlogic.diff
Type: application/octet-stream
Size: 20470 bytes
Desc: incorrectlogic.diff
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140221/6ce52cba/attachment.obj>
More information about the cfe-commits
mailing list