[PATCH] check for Incorrect logic in operator

Anders Rönnholm Anders.Ronnholm at evidente.se
Fri Dec 20 04:35:19 PST 2013


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: text/x-patch
Size: 20470 bytes
Desc: incorrectlogic.diff
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131220/7db884ed/attachment.bin>


More information about the cfe-commits mailing list