[PATCH] D39584: [analyzer] [NFC] another very minor ExprEngineC refactoring

2017-11-09 Thread George Karpenkov via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL317849: [analyzer] [NFC] Minor ExprEngineC refactoring 
(authored by george.karpenkov).

Changed prior to commit:
  https://reviews.llvm.org/D39584?vs=121814=122345#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39584

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp


Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -20,6 +20,24 @@
 using namespace ento;
 using llvm::APSInt;
 
+/// \brief Optionally conjure and return a symbol for offset when processing
+/// an expression \p Expression.
+/// If \p Other is a location, conjure a symbol for \p Symbol
+/// (offset) if it is unknown so that memory arithmetic always
+/// results in an ElementRegion.
+/// \p Count The number of times the current basic block was visited.
+static SVal conjureOffsetSymbolOnLocation(
+SVal Symbol, SVal Other, Expr* Expression, SValBuilder ,
+unsigned Count, const LocationContext *LCtx) {
+  QualType Ty = Expression->getType();
+  if (Other.getAs() &&
+  Ty->isIntegralOrEnumerationType() &&
+  Symbol.isUnknown()) {
+return svalBuilder.conjureSymbolVal(Expression, LCtx, Ty, Count);
+  }
+  return Symbol;
+}
+
 void ExprEngine::VisitBinaryOperator(const BinaryOperator* B,
  ExplodedNode *Pred,
  ExplodedNodeSet ) {
@@ -63,24 +81,13 @@
   StmtNodeBuilder Bldr(*it, Tmp2, *currBldrCtx);
 
   if (B->isAdditiveOp()) {
-// If one of the operands is a location, conjure a symbol for the other
-// one (offset) if it's unknown so that memory arithmetic always
-// results in an ElementRegion.
 // TODO: This can be removed after we enable history tracking with
 // SymSymExpr.
 unsigned Count = currBldrCtx->blockCount();
-if (LeftV.getAs() &&
-RHS->getType()->isIntegralOrEnumerationType() &&
-RightV.isUnknown()) {
-  RightV = svalBuilder.conjureSymbolVal(RHS, LCtx, RHS->getType(),
-Count);
-}
-if (RightV.getAs() &&
-LHS->getType()->isIntegralOrEnumerationType() &&
-LeftV.isUnknown()) {
-  LeftV = svalBuilder.conjureSymbolVal(LHS, LCtx, LHS->getType(),
-   Count);
-}
+RightV = conjureOffsetSymbolOnLocation(
+RightV, LeftV, RHS, svalBuilder, Count, LCtx);
+LeftV = conjureOffsetSymbolOnLocation(
+LeftV, RightV, LHS, svalBuilder, Count, LCtx);
   }
 
   // Although we don't yet model pointers-to-members, we do need to make


Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -20,6 +20,24 @@
 using namespace ento;
 using llvm::APSInt;
 
+/// \brief Optionally conjure and return a symbol for offset when processing
+/// an expression \p Expression.
+/// If \p Other is a location, conjure a symbol for \p Symbol
+/// (offset) if it is unknown so that memory arithmetic always
+/// results in an ElementRegion.
+/// \p Count The number of times the current basic block was visited.
+static SVal conjureOffsetSymbolOnLocation(
+SVal Symbol, SVal Other, Expr* Expression, SValBuilder ,
+unsigned Count, const LocationContext *LCtx) {
+  QualType Ty = Expression->getType();
+  if (Other.getAs() &&
+  Ty->isIntegralOrEnumerationType() &&
+  Symbol.isUnknown()) {
+return svalBuilder.conjureSymbolVal(Expression, LCtx, Ty, Count);
+  }
+  return Symbol;
+}
+
 void ExprEngine::VisitBinaryOperator(const BinaryOperator* B,
  ExplodedNode *Pred,
  ExplodedNodeSet ) {
@@ -63,24 +81,13 @@
   StmtNodeBuilder Bldr(*it, Tmp2, *currBldrCtx);
 
   if (B->isAdditiveOp()) {
-// If one of the operands is a location, conjure a symbol for the other
-// one (offset) if it's unknown so that memory arithmetic always
-// results in an ElementRegion.
 // TODO: This can be removed after we enable history tracking with
 // SymSymExpr.
 unsigned Count = currBldrCtx->blockCount();
-if (LeftV.getAs() &&
-RHS->getType()->isIntegralOrEnumerationType() &&
-RightV.isUnknown()) {
-  RightV = svalBuilder.conjureSymbolVal(RHS, LCtx, RHS->getType(),
-Count);
-}
-if (RightV.getAs() &&
-LHS->getType()->isIntegralOrEnumerationType() &&
-LeftV.isUnknown()) {
-  LeftV =

[PATCH] D39584: [analyzer] [NFC] another very minor ExprEngineC refactoring

2017-11-09 Thread Devin Coughlin via Phabricator via cfe-commits 
dcoughlin accepted this revision.
dcoughlin added a comment.
This revision is now accepted and ready to land.

Yes, looks great. Thanks!


https://reviews.llvm.org/D39584



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

[PATCH] D39584: [analyzer] [NFC] another very minor ExprEngineC refactoring

2017-11-08 Thread George Karpenkov via Phabricator via cfe-commits 
george.karpenkov added a comment.

@dcoughlin OK to commit?


https://reviews.llvm.org/D39584



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

[PATCH] D39584: [analyzer] [NFC] another very minor ExprEngineC refactoring

2017-11-06 Thread George Karpenkov via Phabricator via cfe-commits 
george.karpenkov updated this revision to Diff 121814.
george.karpenkov edited the summary of this revision.
george.karpenkov added a comment.

Addressed review comments.


https://reviews.llvm.org/D39584

Files:
  lib/StaticAnalyzer/Core/ExprEngineC.cpp


Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -20,6 +20,24 @@
 using namespace ento;
 using llvm::APSInt;
 
+/// \brief Optionally conjure and return a symbol for offset when processing
+/// an expression \p Expression.
+/// If \p Other is a location, conjure a symbol for \p Symbol
+/// (offset) if it is unknown so that memory arithmetic always
+/// results in an ElementRegion.
+/// \p Count The number of times the current basic block was visited.
+static SVal conjureOffsetSymbolOnLocation(
+SVal Symbol, SVal Other, Expr* Expression, SValBuilder ,
+unsigned Count, const LocationContext *LCtx) {
+  QualType Ty = Expression->getType();
+  if (Other.getAs() &&
+  Ty->isIntegralOrEnumerationType() &&
+  Symbol.isUnknown()) {
+return svalBuilder.conjureSymbolVal(Expression, LCtx, Ty, Count);
+  }
+  return Symbol;
+}
+
 void ExprEngine::VisitBinaryOperator(const BinaryOperator* B,
  ExplodedNode *Pred,
  ExplodedNodeSet ) {
@@ -63,24 +81,13 @@
   StmtNodeBuilder Bldr(*it, Tmp2, *currBldrCtx);
 
   if (B->isAdditiveOp()) {
-// If one of the operands is a location, conjure a symbol for the other
-// one (offset) if it's unknown so that memory arithmetic always
-// results in an ElementRegion.
 // TODO: This can be removed after we enable history tracking with
 // SymSymExpr.
 unsigned Count = currBldrCtx->blockCount();
-if (LeftV.getAs() &&
-RHS->getType()->isIntegralOrEnumerationType() &&
-RightV.isUnknown()) {
-  RightV = svalBuilder.conjureSymbolVal(RHS, LCtx, RHS->getType(),
-Count);
-}
-if (RightV.getAs() &&
-LHS->getType()->isIntegralOrEnumerationType() &&
-LeftV.isUnknown()) {
-  LeftV = svalBuilder.conjureSymbolVal(LHS, LCtx, LHS->getType(),
-   Count);
-}
+RightV = conjureOffsetSymbolOnLocation(
+RightV, LeftV, RHS, svalBuilder, Count, LCtx);
+LeftV = conjureOffsetSymbolOnLocation(
+LeftV, RightV, LHS, svalBuilder, Count, LCtx);
   }
 
   // Although we don't yet model pointers-to-members, we do need to make


Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -20,6 +20,24 @@
 using namespace ento;
 using llvm::APSInt;
 
+/// \brief Optionally conjure and return a symbol for offset when processing
+/// an expression \p Expression.
+/// If \p Other is a location, conjure a symbol for \p Symbol
+/// (offset) if it is unknown so that memory arithmetic always
+/// results in an ElementRegion.
+/// \p Count The number of times the current basic block was visited.
+static SVal conjureOffsetSymbolOnLocation(
+SVal Symbol, SVal Other, Expr* Expression, SValBuilder ,
+unsigned Count, const LocationContext *LCtx) {
+  QualType Ty = Expression->getType();
+  if (Other.getAs() &&
+  Ty->isIntegralOrEnumerationType() &&
+  Symbol.isUnknown()) {
+return svalBuilder.conjureSymbolVal(Expression, LCtx, Ty, Count);
+  }
+  return Symbol;
+}
+
 void ExprEngine::VisitBinaryOperator(const BinaryOperator* B,
  ExplodedNode *Pred,
  ExplodedNodeSet ) {
@@ -63,24 +81,13 @@
   StmtNodeBuilder Bldr(*it, Tmp2, *currBldrCtx);
 
   if (B->isAdditiveOp()) {
-// If one of the operands is a location, conjure a symbol for the other
-// one (offset) if it's unknown so that memory arithmetic always
-// results in an ElementRegion.
 // TODO: This can be removed after we enable history tracking with
 // SymSymExpr.
 unsigned Count = currBldrCtx->blockCount();
-if (LeftV.getAs() &&
-RHS->getType()->isIntegralOrEnumerationType() &&
-RightV.isUnknown()) {
-  RightV = svalBuilder.conjureSymbolVal(RHS, LCtx, RHS->getType(),
-Count);
-}
-if (RightV.getAs() &&
-LHS->getType()->isIntegralOrEnumerationType() &&
-LeftV.isUnknown()) {
-  LeftV = svalBuilder.conjureSymbolVal(LHS, LCtx, LHS->getType(),
-   Count);
-}
+RightV = conjureOffsetSymbolOnLocation(
+RightV,

[PATCH] D39584: [analyzer] [NFC] another very minor ExprEngineC refactoring

2017-11-06 Thread Devin Coughlin via Phabricator via cfe-commits 
dcoughlin added inline comments.



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:26
+/// results in an ElementRegion.
+static void conjureOffsetSymbolOnLocation(
+SVal , SVal Other, Expr* Expression, SValBuilder ,

I think it would be more clear at the call site what the inputs and output are 
if this function returned the conjured SVal (or 'Symbol' if it wouldn't be 
conjured).

It would also be good to describe the role of the parameters in the doxygen 
comment.


https://reviews.llvm.org/D39584



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

[PATCH] D39584: [analyzer] [NFC] another very minor ExprEngineC refactoring

2017-11-02 Thread George Karpenkov via Phabricator via cfe-commits 
george.karpenkov created this revision.
Herald added subscribers: szepet, xazax.hun.

couldn't resist adding a function here.
I think this would be the last one.


https://reviews.llvm.org/D39584

Files:
  lib/StaticAnalyzer/Core/ExprEngineC.cpp


Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -20,6 +20,20 @@
 using namespace ento;
 using llvm::APSInt;
 
+/// If one of the operands is a location, conjure a symbol for the other
+/// one (offset) if it's unknown so that memory arithmetic always
+/// results in an ElementRegion.
+static void conjureOffsetSymbolOnLocation(
+SVal , SVal Other, Expr* Expression, SValBuilder ,
+unsigned Count, const LocationContext *LCtx) {
+  QualType Ty = Expression->getType();
+  if (Other.getAs() &&
+  Ty->isIntegralOrEnumerationType() &&
+  Symbol.isUnknown()) {
+Symbol = svalBuilder.conjureSymbolVal(Expression, LCtx, Ty, Count);
+  }
+}
+
 void ExprEngine::VisitBinaryOperator(const BinaryOperator* B,
  ExplodedNode *Pred,
  ExplodedNodeSet ) {
@@ -63,24 +77,13 @@
   StmtNodeBuilder Bldr(*it, Tmp2, *currBldrCtx);
 
   if (B->isAdditiveOp()) {
-// If one of the operands is a location, conjure a symbol for the other
-// one (offset) if it's unknown so that memory arithmetic always
-// results in an ElementRegion.
 // TODO: This can be removed after we enable history tracking with
 // SymSymExpr.
 unsigned Count = currBldrCtx->blockCount();
-if (LeftV.getAs() &&
-RHS->getType()->isIntegralOrEnumerationType() &&
-RightV.isUnknown()) {
-  RightV = svalBuilder.conjureSymbolVal(RHS, LCtx, RHS->getType(),
-Count);
-}
-if (RightV.getAs() &&
-LHS->getType()->isIntegralOrEnumerationType() &&
-LeftV.isUnknown()) {
-  LeftV = svalBuilder.conjureSymbolVal(LHS, LCtx, LHS->getType(),
-   Count);
-}
+conjureOffsetSymbolOnLocation(
+RightV, LeftV, RHS, svalBuilder, Count, LCtx);
+conjureOffsetSymbolOnLocation(
+LeftV, RightV, LHS, svalBuilder, Count, LCtx);
   }
 
   // Although we don't yet model pointers-to-members, we do need to make


Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -20,6 +20,20 @@
 using namespace ento;
 using llvm::APSInt;
 
+/// If one of the operands is a location, conjure a symbol for the other
+/// one (offset) if it's unknown so that memory arithmetic always
+/// results in an ElementRegion.
+static void conjureOffsetSymbolOnLocation(
+SVal , SVal Other, Expr* Expression, SValBuilder ,
+unsigned Count, const LocationContext *LCtx) {
+  QualType Ty = Expression->getType();
+  if (Other.getAs() &&
+  Ty->isIntegralOrEnumerationType() &&
+  Symbol.isUnknown()) {
+Symbol = svalBuilder.conjureSymbolVal(Expression, LCtx, Ty, Count);
+  }
+}
+
 void ExprEngine::VisitBinaryOperator(const BinaryOperator* B,
  ExplodedNode *Pred,
  ExplodedNodeSet ) {
@@ -63,24 +77,13 @@
   StmtNodeBuilder Bldr(*it, Tmp2, *currBldrCtx);
 
   if (B->isAdditiveOp()) {
-// If one of the operands is a location, conjure a symbol for the other
-// one (offset) if it's unknown so that memory arithmetic always
-// results in an ElementRegion.
 // TODO: This can be removed after we enable history tracking with
 // SymSymExpr.
 unsigned Count = currBldrCtx->blockCount();
-if (LeftV.getAs() &&
-RHS->getType()->isIntegralOrEnumerationType() &&
-RightV.isUnknown()) {
-  RightV = svalBuilder.conjureSymbolVal(RHS, LCtx, RHS->getType(),
-Count);
-}
-if (RightV.getAs() &&
-LHS->getType()->isIntegralOrEnumerationType() &&
-LeftV.isUnknown()) {
-  LeftV = svalBuilder.conjureSymbolVal(LHS, LCtx, LHS->getType(),
-   Count);
-}
+conjureOffsetSymbolOnLocation(
+RightV, LeftV, RHS, svalBuilder, Count, LCtx);
+conjureOffsetSymbolOnLocation(
+LeftV, RightV, LHS, svalBuilder, Count, LCtx);
   }
 
   // Although we don't yet model pointers-to-members, we do need to make
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits