[PATCH] D26061: [analyzer] Refactor and simplify SimpleConstraintManager

[Dominic Chen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL296242: [analyzer] Refactor and simplify 
SimpleConstraintManager (authored by ddcc).

Changed prior to commit:
  https://reviews.llvm.org/D26061?vs=89054=89773#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D26061

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h
  cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt
  cfe/trunk/lib/StaticAnalyzer/Core/ConstraintManager.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/RangedConstraintManager.h
  cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.h

Index: cfe/trunk/lib/StaticAnalyzer/Core/ConstraintManager.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ConstraintManager.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ConstraintManager.cpp
@@ -20,8 +20,8 @@
 
 static DefinedSVal getLocFromSymbol(const ProgramStateRef ,
 SymbolRef Sym) {
-  const MemRegion *R = State->getStateManager().getRegionManager()
-   .getSymbolicRegion(Sym);
+  const MemRegion *R =
+  State->getStateManager().getRegionManager().getSymbolicRegion(Sym);
   return loc::MemRegionVal(R);
 }
 
Index: cfe/trunk/lib/StaticAnalyzer/Core/RangedConstraintManager.h
===
--- cfe/trunk/lib/StaticAnalyzer/Core/RangedConstraintManager.h
+++ cfe/trunk/lib/StaticAnalyzer/Core/RangedConstraintManager.h
@@ -0,0 +1,102 @@
+//== RangedConstraintManager.h --*- C++ -*--==//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+//  Ranged constraint manager, built on SimpleConstraintManager.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_STATICANALYZER_CORE_RANGEDCONSTRAINTMANAGER_H
+#define LLVM_CLANG_LIB_STATICANALYZER_CORE_RANGEDCONSTRAINTMANAGER_H
+
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h"
+
+namespace clang {
+
+namespace ento {
+
+class RangedConstraintManager : public SimpleConstraintManager {
+public:
+  RangedConstraintManager(SubEngine *SE, SValBuilder )
+  : SimpleConstraintManager(SE, SB) {}
+
+  ~RangedConstraintManager() override;
+
+  //===--===//
+  // Implementation for interface from SimpleConstraintManager.
+  //===--===//
+
+  ProgramStateRef assumeSym(ProgramStateRef State, SymbolRef Sym,
+bool Assumption) override;
+
+  ProgramStateRef assumeSymInclusiveRange(ProgramStateRef State, SymbolRef Sym,
+  const llvm::APSInt ,
+  const llvm::APSInt ,
+  bool InRange) override;
+
+  ProgramStateRef assumeSymUnsupported(ProgramStateRef State, SymbolRef Sym,
+   bool Assumption) override;
+
+protected:
+  /// Assume a constraint between a symbolic expression and a concrete integer.
+  virtual ProgramStateRef assumeSymRel(ProgramStateRef State, SymbolRef Sym,
+   BinaryOperator::Opcode op,
+   const llvm::APSInt );
+
+  //===--===//
+  // Interface that subclasses must implement.
+  //===--===//
+
+  // Each of these is of the form "$Sym+Adj <> V", where "<>" is the comparison
+  // operation for the method being invoked.
+
+  virtual ProgramStateRef assumeSymNE(ProgramStateRef State, SymbolRef Sym,
+  const llvm::APSInt ,
+  const llvm::APSInt ) = 0;
+
+  virtual ProgramStateRef assumeSymEQ(ProgramStateRef State, SymbolRef Sym,
+  const llvm::APSInt ,
+  const llvm::APSInt ) = 0;
+
+  virtual ProgramStateRef assumeSymLT(ProgramStateRef State, SymbolRef Sym,
+  const llvm::APSInt ,
+  const llvm::APSInt ) = 0;
+
+  virtual ProgramStateRef assumeSymGT(ProgramStateRef State, 

[PATCH] D26061: [analyzer] Refactor and simplify SimpleConstraintManager

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 89054.
ddcc added a comment.

Rebase, incorporate https://reviews.llvm.org/D22862


https://reviews.llvm.org/D26061

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h
  lib/StaticAnalyzer/Core/CMakeLists.txt
  lib/StaticAnalyzer/Core/ConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.h
  lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  lib/StaticAnalyzer/Core/SimpleConstraintManager.h

Index: lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
@@ -7,12 +7,12 @@
 //
 //===--===//
 //
-//  This file defines SimpleConstraintManager, a class that holds code shared
-//  between BasicConstraintManager and RangeConstraintManager.
+//  This file defines SimpleConstraintManager, a class that provides a
+//  simplified constraint manager interface, compared to ConstraintManager.
 //
 //===--===//
 
-#include "SimpleConstraintManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
@@ -23,48 +23,6 @@
 
 SimpleConstraintManager::~SimpleConstraintManager() {}
 
-bool SimpleConstraintManager::canReasonAbout(SVal X) const {
-  Optional SymVal = X.getAs();
-  if (SymVal && SymVal->isExpression()) {
-const SymExpr *SE = SymVal->getSymbol();
-
-if (const SymIntExpr *SIE = dyn_cast(SE)) {
-  switch (SIE->getOpcode()) {
-  // We don't reason yet about bitwise-constraints on symbolic values.
-  case BO_And:
-  case BO_Or:
-  case BO_Xor:
-return false;
-  // We don't reason yet about these arithmetic constraints on
-  // symbolic values.
-  case BO_Mul:
-  case BO_Div:
-  case BO_Rem:
-  case BO_Shl:
-  case BO_Shr:
-return false;
-  // All other cases.
-  default:
-return true;
-  }
-}
-
-if (const SymSymExpr *SSE = dyn_cast(SE)) {
-  if (BinaryOperator::isComparisonOp(SSE->getOpcode())) {
-// We handle Loc <> Loc comparisons, but not (yet) NonLoc <> NonLoc.
-if (Loc::isLocType(SSE->getLHS()->getType())) {
-  assert(Loc::isLocType(SSE->getRHS()->getType()));
-  return true;
-}
-  }
-}
-
-return false;
-  }
-
-  return true;
-}
-
 ProgramStateRef SimpleConstraintManager::assume(ProgramStateRef State,
 DefinedSVal Cond,
 bool Assumption) {
@@ -92,23 +50,6 @@
   return State;
 }
 
-ProgramStateRef
-SimpleConstraintManager::assumeAuxForSymbol(ProgramStateRef State,
-SymbolRef Sym, bool Assumption) {
-  BasicValueFactory  = getBasicVals();
-  QualType T = Sym->getType();
-
-  // None of the constraint solvers currently support non-integer types.
-  if (!T->isIntegralOrEnumerationType())
-return State;
-
-  const llvm::APSInt  = BVF.getValue(0, T);
-  if (Assumption)
-return assumeSymNE(State, Sym, zero, zero);
-  else
-return assumeSymEQ(State, Sym, zero, zero);
-}
-
 ProgramStateRef SimpleConstraintManager::assumeAux(ProgramStateRef State,
NonLoc Cond,
bool Assumption) {
@@ -118,7 +59,8 @@
   if (!canReasonAbout(Cond)) {
 // Just add the constraint to the expression without trying to simplify.
 SymbolRef Sym = Cond.getAsSymExpr();
-return assumeAuxForSymbol(State, Sym, Assumption);
+assert(Sym);
+return assumeSymUnsupported(State, Sym, Assumption);
   }
 
   switch (Cond.getSubKind()) {
@@ -129,51 +71,7 @@
 nonloc::SymbolVal SV = Cond.castAs();
 SymbolRef Sym = SV.getSymbol();
 assert(Sym);
-
-// Handle SymbolData.
-if (!SV.isExpression()) {
-  return assumeAuxForSymbol(State, Sym, Assumption);
-
-  // Handle symbolic expression.
-} else if (const SymIntExpr *SE = dyn_cast(Sym)) {
-  // We can only simplify expressions whose RHS is an integer.
-
-  BinaryOperator::Opcode Op = SE->getOpcode();
-  if (BinaryOperator::isComparisonOp(Op)) {
-if (!Assumption)
-  Op = BinaryOperator::negateComparisonOp(Op);
-
-return assumeSymRel(State, SE->getLHS(), Op, SE->getRHS());
-  }
-
-} else if (const SymSymExpr *SSE = dyn_cast(Sym)) {
-  

[PATCH] D26061: [analyzer] Refactor and simplify SimpleConstraintManager

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 85136.
ddcc added a comment.

Rebase


https://reviews.llvm.org/D26061

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h
  lib/StaticAnalyzer/Core/CMakeLists.txt
  lib/StaticAnalyzer/Core/ConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.h
  lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  lib/StaticAnalyzer/Core/SimpleConstraintManager.h

Index: lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
@@ -7,12 +7,12 @@
 //
 //===--===//
 //
-//  This file defines SimpleConstraintManager, a class that holds code shared
-//  between BasicConstraintManager and RangeConstraintManager.
+//  This file defines SimpleConstraintManager, a class that provides a
+//  simplified constraint manager interface, compared to ConstraintManager.
 //
 //===--===//
 
-#include "SimpleConstraintManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
@@ -23,48 +23,6 @@
 
 SimpleConstraintManager::~SimpleConstraintManager() {}
 
-bool SimpleConstraintManager::canReasonAbout(SVal X) const {
-  Optional SymVal = X.getAs();
-  if (SymVal && SymVal->isExpression()) {
-const SymExpr *SE = SymVal->getSymbol();
-
-if (const SymIntExpr *SIE = dyn_cast(SE)) {
-  switch (SIE->getOpcode()) {
-  // We don't reason yet about bitwise-constraints on symbolic values.
-  case BO_And:
-  case BO_Or:
-  case BO_Xor:
-return false;
-  // We don't reason yet about these arithmetic constraints on
-  // symbolic values.
-  case BO_Mul:
-  case BO_Div:
-  case BO_Rem:
-  case BO_Shl:
-  case BO_Shr:
-return false;
-  // All other cases.
-  default:
-return true;
-  }
-}
-
-if (const SymSymExpr *SSE = dyn_cast(SE)) {
-  if (BinaryOperator::isComparisonOp(SSE->getOpcode())) {
-// We handle Loc <> Loc comparisons, but not (yet) NonLoc <> NonLoc.
-if (Loc::isLocType(SSE->getLHS()->getType())) {
-  assert(Loc::isLocType(SSE->getRHS()->getType()));
-  return true;
-}
-  }
-}
-
-return false;
-  }
-
-  return true;
-}
-
 ProgramStateRef SimpleConstraintManager::assume(ProgramStateRef State,
 DefinedSVal Cond,
 bool Assumption) {
@@ -92,23 +50,6 @@
   return State;
 }
 
-ProgramStateRef
-SimpleConstraintManager::assumeAuxForSymbol(ProgramStateRef State,
-SymbolRef Sym, bool Assumption) {
-  BasicValueFactory  = getBasicVals();
-  QualType T = Sym->getType();
-
-  // None of the constraint solvers currently support non-integer types.
-  if (!T->isIntegralOrEnumerationType())
-return State;
-
-  const llvm::APSInt  = BVF.getValue(0, T);
-  if (Assumption)
-return assumeSymNE(State, Sym, zero, zero);
-  else
-return assumeSymEQ(State, Sym, zero, zero);
-}
-
 ProgramStateRef SimpleConstraintManager::assumeAux(ProgramStateRef State,
NonLoc Cond,
bool Assumption) {
@@ -118,7 +59,8 @@
   if (!canReasonAbout(Cond)) {
 // Just add the constraint to the expression without trying to simplify.
 SymbolRef Sym = Cond.getAsSymExpr();
-return assumeAuxForSymbol(State, Sym, Assumption);
+assert(Sym);
+return assumeSymUnsupported(State, Sym, Assumption);
   }
 
   switch (Cond.getSubKind()) {
@@ -129,51 +71,7 @@
 nonloc::SymbolVal SV = Cond.castAs();
 SymbolRef Sym = SV.getSymbol();
 assert(Sym);
-
-// Handle SymbolData.
-if (!SV.isExpression()) {
-  return assumeAuxForSymbol(State, Sym, Assumption);
-
-  // Handle symbolic expression.
-} else if (const SymIntExpr *SE = dyn_cast(Sym)) {
-  // We can only simplify expressions whose RHS is an integer.
-
-  BinaryOperator::Opcode Op = SE->getOpcode();
-  if (BinaryOperator::isComparisonOp(Op)) {
-if (!Assumption)
-  Op = BinaryOperator::negateComparisonOp(Op);
-
-return assumeSymRel(State, SE->getLHS(), Op, SE->getRHS());
-  }
-
-} else if (const SymSymExpr *SSE = dyn_cast(Sym)) {
-  // Translate "a != b" to "(b - a) != 0".
- 

[PATCH] D26061: [analyzer] Refactor and simplify SimpleConstraintManager

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

How does this look now?


https://reviews.llvm.org/D26061



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26061: [analyzer] Refactor and simplify SimpleConstraintManager

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 80129.
ddcc added a comment.

Rebase, move `assumeSymRel()` to RangedConstraintManager, make 
`assumeSymUnsupported` pure virtual in SimpleConstraintManager.


https://reviews.llvm.org/D26061

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h
  lib/StaticAnalyzer/Core/CMakeLists.txt
  lib/StaticAnalyzer/Core/ConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.h
  lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  lib/StaticAnalyzer/Core/SimpleConstraintManager.h

Index: lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
@@ -7,12 +7,12 @@
 //
 //===--===//
 //
-//  This file defines SimpleConstraintManager, a class that holds code shared
-//  between BasicConstraintManager and RangeConstraintManager.
+//  This file defines SimpleConstraintManager, a class that provides a
+//  simplified constraint manager interface, compared to ConstraintManager.
 //
 //===--===//
 
-#include "SimpleConstraintManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
@@ -23,48 +23,6 @@
 
 SimpleConstraintManager::~SimpleConstraintManager() {}
 
-bool SimpleConstraintManager::canReasonAbout(SVal X) const {
-  Optional SymVal = X.getAs();
-  if (SymVal && SymVal->isExpression()) {
-const SymExpr *SE = SymVal->getSymbol();
-
-if (const SymIntExpr *SIE = dyn_cast(SE)) {
-  switch (SIE->getOpcode()) {
-  // We don't reason yet about bitwise-constraints on symbolic values.
-  case BO_And:
-  case BO_Or:
-  case BO_Xor:
-return false;
-  // We don't reason yet about these arithmetic constraints on
-  // symbolic values.
-  case BO_Mul:
-  case BO_Div:
-  case BO_Rem:
-  case BO_Shl:
-  case BO_Shr:
-return false;
-  // All other cases.
-  default:
-return true;
-  }
-}
-
-if (const SymSymExpr *SSE = dyn_cast(SE)) {
-  if (BinaryOperator::isComparisonOp(SSE->getOpcode())) {
-// We handle Loc <> Loc comparisons, but not (yet) NonLoc <> NonLoc.
-if (Loc::isLocType(SSE->getLHS()->getType())) {
-  assert(Loc::isLocType(SSE->getRHS()->getType()));
-  return true;
-}
-  }
-}
-
-return false;
-  }
-
-  return true;
-}
-
 ProgramStateRef SimpleConstraintManager::assume(ProgramStateRef State,
 DefinedSVal Cond,
 bool Assumption) {
@@ -92,23 +50,6 @@
   return State;
 }
 
-ProgramStateRef
-SimpleConstraintManager::assumeAuxForSymbol(ProgramStateRef State,
-SymbolRef Sym, bool Assumption) {
-  BasicValueFactory  = getBasicVals();
-  QualType T = Sym->getType();
-
-  // None of the constraint solvers currently support non-integer types.
-  if (!T->isIntegralOrEnumerationType())
-return State;
-
-  const llvm::APSInt  = BVF.getValue(0, T);
-  if (Assumption)
-return assumeSymNE(State, Sym, zero, zero);
-  else
-return assumeSymEQ(State, Sym, zero, zero);
-}
-
 ProgramStateRef SimpleConstraintManager::assumeAux(ProgramStateRef State,
NonLoc Cond,
bool Assumption) {
@@ -118,7 +59,8 @@
   if (!canReasonAbout(Cond)) {
 // Just add the constraint to the expression without trying to simplify.
 SymbolRef Sym = Cond.getAsSymExpr();
-return assumeAuxForSymbol(State, Sym, Assumption);
+assert(Sym);
+return assumeSymUnsupported(State, Sym, Assumption);
   }
 
   switch (Cond.getSubKind()) {
@@ -129,51 +71,7 @@
 nonloc::SymbolVal SV = Cond.castAs();
 SymbolRef Sym = SV.getSymbol();
 assert(Sym);
-
-// Handle SymbolData.
-if (!SV.isExpression()) {
-  return assumeAuxForSymbol(State, Sym, Assumption);
-
-  // Handle symbolic expression.
-} else if (const SymIntExpr *SE = dyn_cast(Sym)) {
-  // We can only simplify expressions whose RHS is an integer.
-
-  BinaryOperator::Opcode Op = SE->getOpcode();
-  if (BinaryOperator::isComparisonOp(Op)) {
-if (!Assumption)
-  Op = BinaryOperator::negateComparisonOp(Op);
-
-return assumeSymRel(State, SE->getLHS(), Op, SE->getRHS());

[PATCH] D26061: [analyzer] Refactor and simplify SimpleConstraintManager

[Dominic Chen via cfe-commits]

ddcc updated this revision to Diff 78049.
ddcc added a comment.

Rebase on recent changes


https://reviews.llvm.org/D26061

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h
  lib/StaticAnalyzer/Core/CMakeLists.txt
  lib/StaticAnalyzer/Core/ConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.h
  lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  lib/StaticAnalyzer/Core/SimpleConstraintManager.h

Index: lib/StaticAnalyzer/Core/SimpleConstraintManager.h
===
--- lib/StaticAnalyzer/Core/SimpleConstraintManager.h
+++ /dev/null
@@ -1,121 +0,0 @@
-//== SimpleConstraintManager.h --*- C++ -*--==//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-//
-//  Code shared between BasicConstraintManager and RangeConstraintManager.
-//
-//===--===//
-
-#ifndef LLVM_CLANG_LIB_STATICANALYZER_CORE_SIMPLECONSTRAINTMANAGER_H
-#define LLVM_CLANG_LIB_STATICANALYZER_CORE_SIMPLECONSTRAINTMANAGER_H
-
-#include "clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
-
-namespace clang {
-
-namespace ento {
-
-class SimpleConstraintManager : public ConstraintManager {
-  SubEngine *SU;
-  SValBuilder 
-public:
-  SimpleConstraintManager(SubEngine *subengine, SValBuilder )
-: SU(subengine), SVB(SB) {}
-  ~SimpleConstraintManager() override;
-
-  //===--===//
-  // Common implementation for the interface provided by ConstraintManager.
-  //===--===//
-
-  ProgramStateRef assume(ProgramStateRef state, DefinedSVal Cond,
-bool Assumption) override;
-
-  ProgramStateRef assume(ProgramStateRef state, NonLoc Cond, bool Assumption);
-
-  ProgramStateRef assumeInclusiveRange(ProgramStateRef State,
- NonLoc Value,
- const llvm::APSInt ,
- const llvm::APSInt ,
- bool InRange) override;
-
-  ProgramStateRef assumeSymRel(ProgramStateRef state,
-  const SymExpr *LHS,
-  BinaryOperator::Opcode op,
-  const llvm::APSInt& Int);
-
-  ProgramStateRef assumeSymWithinInclusiveRange(ProgramStateRef State,
-SymbolRef Sym,
-const llvm::APSInt ,
-const llvm::APSInt ,
-bool InRange);
-
-
-protected:
-
-  //===--===//
-  // Interface that subclasses must implement.
-  //===--===//
-
-  // Each of these is of the form "$sym+Adj <> V", where "<>" is the comparison
-  // operation for the method being invoked.
-  virtual ProgramStateRef assumeSymNE(ProgramStateRef state, SymbolRef sym,
- const llvm::APSInt& V,
- const llvm::APSInt& Adjustment) = 0;
-
-  virtual ProgramStateRef assumeSymEQ(ProgramStateRef state, SymbolRef sym,
- const llvm::APSInt& V,
- const llvm::APSInt& Adjustment) = 0;
-
-  virtual ProgramStateRef assumeSymLT(ProgramStateRef state, SymbolRef sym,
- const llvm::APSInt& V,
- const llvm::APSInt& Adjustment) = 0;
-
-  virtual ProgramStateRef assumeSymGT(ProgramStateRef state, SymbolRef sym,
- const llvm::APSInt& V,
- const llvm::APSInt& Adjustment) = 0;
-
-  virtual ProgramStateRef assumeSymLE(ProgramStateRef state, SymbolRef sym,
- const llvm::APSInt& V,
- const llvm::APSInt& Adjustment) = 0;
-
-  virtual ProgramStateRef assumeSymGE(ProgramStateRef state, SymbolRef sym,
- const llvm::APSInt& V,
- const llvm::APSInt& Adjustment) = 0;
-
-
-  virtual ProgramStateRef assumeSymbolWithinInclusiveRange(
-  ProgramStateRef State, 

[PATCH] D26061: [analyzer] Refactor and simplify SimpleConstraintManager

[Dominic Chen via cfe-commits]

ddcc added a comment.

Yes, I've been writing a Z3 solver interface, which motivated this patch. 
However, this patch has snowballed into something that it's a little too 
convoluted, so I'll split it up.

I'm not sure whether the RangedConstraintManager interface is useful or not; I 
preserved it because it's currently in the code, but since 
RangeConstraintManager is the only user, it is possible to merge the two 
together and eliminate the interface. In the past, BasicConstraintManager was 
the other class that used this interface, but that was deleted quite a while 
back, and I'm not sure if there are plans for anything else?


https://reviews.llvm.org/D26061



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26061: [analyzer] Refactor and simplify SimpleConstraintManager

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

Thanks for the patch!

Would it be possible to split this up into several patches? I think it is 
important to separate the interface layering changes from the formatting 
changes, renaming changes, and minor optimization changes. This will make the 
patches easier to review.

Also: is this motivated by a different implementation of range constraints? Is 
this something you plan on upstreaming?

In https://reviews.llvm.org/D26061#581778, @ddcc wrote:

> To summarize, here is a list of changes:
>
> - General
>   - Fixed some issues with formatting (`clang-format`)
>   - Fixed inconsistent capitalization following camel case style guidelines
> - `ConstraintManager.h`
>   - Renamed `assumeWithinInclusiveRange*()` to `assumeInclusiveRange*()`, 
> since the range is not necessarily contained within unless `Assumption` is 
> true, to match `assume()`
> - `RangedConstraintManager.h` (inherits `SimpleConstraintManager`)
>   - Moved `assumeSym*` and `canReasonAbout()` from `SimpleConstraintManager` 
> to here
>   - Renamed 
> `assumeSymbolWithinInclusiveRange`/`assumeSymbolOutOfInclusiveRange` to 
> `assumeSymWithinInclusiveRange`/`assumeSymOutsideInclusiveRange` to match the 
> above, and moved to here
>   - This is now inherited by `RangeConstraintManager`, and essentially 
> provides the current interface in `SimpleConstraintManager`


What do you view as the distinction between RangeConstraintMananger and 
RangedConstraintManager? In other words, if I'm a developer adding new 
functionality how would you suggest I decide which to add it to? It would be 
good to document this in the code! Also: the closeness in the names of these 
classes could be potentially confusing. Are there other, more distinctive, 
names that still communicate the difference you intend?

> 
> 
> - `SimpleConstraintManager.h` (inherits `ConstraintManager`)
>   - Implemented a new interface that internally converts the `DefinedSVal` in 
> `assume(...)` from `ConstraintManager.h` to `NonLoc` to `SymbolRef`. 
> Subclasses only need to override `ProgramStateRef assumeSym(ProgramStateRef 
> State, SymbolRef Sym, bool Assumption)`
>   - For inclusive ranges, implemented a new interface that internally 
> converts from `NonLoc` to `SymbolRef`. Subclasses only need to override 
> `ProgramStateRef assumeSymInclusiveRange(ProgramStateRef State, SymbolRef 
> Sym, const llvm::APSInt , const llvm::APSInt , bool InRange)`
>   - Implemented a new interface that internally handles expressions that fail 
> `canReasonAbout()` by adding them directly to the constraint manager state. 
> Subclasses only need to expose `ProgramStateRef assumeSymRel(ProgramStateRef 
> State, SymbolRef Sym, BinaryOperator::Opcode op, const llvm::APSInt )`
> - `RangeConstraintManager.cpp`
>   - Minor optimization to avoid updating the state if nothing is pruned in 
> `removeDeadBindings()`


https://reviews.llvm.org/D26061



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26061: [analyzer] Refactor and simplify SimpleConstraintManager

[Dominic Chen via cfe-commits]

ddcc added a comment.

To summarize, here is a list of changes:

- General
  - Fixed some issues with formatting (`clang-format`)
  - Fixed inconsistent capitalization following camel case style guidelines

- `ConstraintManager.h`
  - Renamed `assumeWithinInclusiveRange*()` to `assumeInclusiveRange*()`, since 
the range is not necessarily inclusive unless `Assumption` is true, to match 
`assume()`

- `RangedConstraintManager.h` (inherits `SimpleConstraintManager`)
  - Moved `assumeSym*` and `canReasonAbout()` from `SimpleConstraintManager` to 
here
  - Renamed 
`assumeSymbolWithinInclusiveRange`/`assumeSymbolOutOfInclusiveRange` to 
`assumeSymWithinInclusiveRange`/`assumeSymOutsideInclusiveRange` to match the 
above, and moved to here
  - This is now inherited by `RangeConstraintManager`, and essentially provides 
the current interface in `SimpleConstraintManager`

- `SimpleConstraintManager.h` (inherits `ConstraintManager`)
  - Implemented a new interface that internally converts the `DefinedSVal` in 
`assume(...)` from `ConstraintManager.h` to `NonLoc` to `SymbolRef`. Subclasses 
only need to override `ProgramStateRef assumeSym(ProgramStateRef State, 
SymbolRef Sym, bool Assumption)`
  - For inclusive ranges, implemented a new interface that internally converts 
from `NonLoc` to `SymbolRef`. Subclasses only need to override `ProgramStateRef 
assumeSymInclusiveRange(ProgramStateRef State, SymbolRef Sym, const 
llvm::APSInt , const llvm::APSInt , bool InRange)`
  - Implemented a new interface that internally handles expressions that fail 
`canReasonAbout()` by adding them directly to the constraint manager state. 
Subclasses only need to expose `ProgramStateRef assumeSymRel(ProgramStateRef 
State, SymbolRef Sym, BinaryOperator::Opcode op, const llvm::APSInt )`

- `RangeConstraintManager.cpp`
  - Minor optimization to avoid updating the state if nothing is pruned in 
`removeDeadBindings()`


https://reviews.llvm.org/D26061



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26061: [analyzer] Refactor and simplify SimpleConstraintManager

[Dominic Chen via cfe-commits]

ddcc created this revision.
ddcc added reviewers: zaks.anna, dcoughlin.
ddcc added subscribers: cfe-commits, rgov, NoQ, xazax.hun.
Herald added a subscriber: mgorny.

SimpleConstraintManager is difficult to use, and makes assumptions about 
capabilities of the constraint manager. This patch refactors out those portions 
into a new RangedConstraintManager, and also fixes some issues with camel case, 
formatting, and confusing naming.


https://reviews.llvm.org/D26061

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h
  lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  lib/StaticAnalyzer/Core/CMakeLists.txt
  lib/StaticAnalyzer/Core/ConstraintManager.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.h
  lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  lib/StaticAnalyzer/Core/SimpleConstraintManager.h

Index: lib/StaticAnalyzer/Core/SimpleConstraintManager.h
===
--- lib/StaticAnalyzer/Core/SimpleConstraintManager.h
+++ /dev/null
@@ -1,121 +0,0 @@
-//== SimpleConstraintManager.h --*- C++ -*--==//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-//
-//  Code shared between BasicConstraintManager and RangeConstraintManager.
-//
-//===--===//
-
-#ifndef LLVM_CLANG_LIB_STATICANALYZER_CORE_SIMPLECONSTRAINTMANAGER_H
-#define LLVM_CLANG_LIB_STATICANALYZER_CORE_SIMPLECONSTRAINTMANAGER_H
-
-#include "clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
-
-namespace clang {
-
-namespace ento {
-
-class SimpleConstraintManager : public ConstraintManager {
-  SubEngine *SU;
-  SValBuilder 
-public:
-  SimpleConstraintManager(SubEngine *subengine, SValBuilder )
-: SU(subengine), SVB(SB) {}
-  ~SimpleConstraintManager() override;
-
-  //===--===//
-  // Common implementation for the interface provided by ConstraintManager.
-  //===--===//
-
-  ProgramStateRef assume(ProgramStateRef state, DefinedSVal Cond,
-bool Assumption) override;
-
-  ProgramStateRef assume(ProgramStateRef state, NonLoc Cond, bool Assumption);
-
-  ProgramStateRef assumeWithinInclusiveRange(ProgramStateRef State,
- NonLoc Value,
- const llvm::APSInt ,
- const llvm::APSInt ,
- bool InRange) override;
-
-  ProgramStateRef assumeSymRel(ProgramStateRef state,
-  const SymExpr *LHS,
-  BinaryOperator::Opcode op,
-  const llvm::APSInt& Int);
-
-  ProgramStateRef assumeSymWithinInclusiveRange(ProgramStateRef State,
-SymbolRef Sym,
-const llvm::APSInt ,
-const llvm::APSInt ,
-bool InRange);
-
-
-protected:
-
-  //===--===//
-  // Interface that subclasses must implement.
-  //===--===//
-
-  // Each of these is of the form "$sym+Adj <> V", where "<>" is the comparison
-  // operation for the method being invoked.
-  virtual ProgramStateRef assumeSymNE(ProgramStateRef state, SymbolRef sym,
- const llvm::APSInt& V,
- const llvm::APSInt& Adjustment) = 0;
-
-  virtual ProgramStateRef assumeSymEQ(ProgramStateRef state, SymbolRef sym,
- const llvm::APSInt& V,
- const llvm::APSInt& Adjustment) = 0;
-
-  virtual ProgramStateRef assumeSymLT(ProgramStateRef state, SymbolRef sym,
- const llvm::APSInt& V,
- const llvm::APSInt& Adjustment) = 0;
-
-  virtual ProgramStateRef assumeSymGT(ProgramStateRef state, SymbolRef sym,
- const llvm::APSInt& V,
- const llvm::APSInt& Adjustment) = 0;
-
-