[PATCH] D129678: [analyzer][NFC] Tidy up handler-functions in SymbolicRangeInferrer

[Denys Petrov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG82f76c04774f: [analyzer][NFC] Tidy up handler-functions in 
SymbolicRangeInferrer (authored by ASDenysPetrov).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129678/new/

https://reviews.llvm.org/D129678

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp

Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -1213,13 +1213,21 @@
   }
 
   RangeSet VisitSymExpr(SymbolRef Sym) {
-// If we got to this function, the actual type of the symbolic
+if (Optional RS = getRangeForNegatedSym(Sym))
+  return *RS;
+// If we've reached this line, the actual type of the symbolic
 // expression is not supported for advanced inference.
 // In this case, we simply backoff to the default "let's simply
 // infer the range from the expression's type".
 return infer(Sym->getType());
   }
 
+  RangeSet VisitUnarySymExpr(const UnarySymExpr *USE) {
+if (Optional RS = getRangeForNegatedUnarySym(USE))
+  return *RS;
+return infer(USE->getType());
+  }
+
   RangeSet VisitSymIntExpr(const SymIntExpr *Sym) {
 return VisitBinaryOperator(Sym);
   }
@@ -1228,14 +1236,25 @@
 return VisitBinaryOperator(Sym);
   }
 
-  RangeSet VisitSymSymExpr(const SymSymExpr *Sym) {
+  RangeSet VisitSymSymExpr(const SymSymExpr *SSE) {
 return intersect(
 RangeFactory,
+// If Sym is a difference of symbols A - B, then maybe we have range
+// set stored for B - A.
+//
+// If we have range set stored for both A - B and B - A then
+// calculate the effective range set by intersecting the range set
+// for A - B and the negated range set of B - A.
+getRangeForNegatedSymSym(SSE),
+// If Sym is a comparison expression (except <=>),
+// find any other comparisons with the same operands.
+// See function description.
+getRangeForComparisonSymbol(SSE),
 // If Sym is (dis)equality, we might have some information
 // on that in our equality classes data structure.
-getRangeForEqualities(Sym),
+getRangeForEqualities(SSE),
 // And we should always check what we can get from the operands.
-VisitBinaryOperator(Sym));
+VisitBinaryOperator(SSE));
   }
 
 private:
@@ -1264,25 +1283,13 @@
   }
 
   RangeSet infer(SymbolRef Sym) {
-return intersect(
-RangeFactory,
-// Of course, we should take the constraint directly associated with
-// this symbol into consideration.
-getConstraint(State, Sym),
-// If Sym is a difference of symbols A - B, then maybe we have range
-// set stored for B - A.
-//
-// If we have range set stored for both A - B and B - A then
-// calculate the effective range set by intersecting the range set
-// for A - B and the negated range set of B - A.
-getRangeForNegatedSub(Sym),
-// If Sym is a comparison expression (except <=>),
-// find any other comparisons with the same operands.
-// See function description.
-getRangeForComparisonSymbol(Sym),
-// Apart from the Sym itself, we can infer quite a lot if we look
-// into subexpressions of Sym.
-Visit(Sym));
+return intersect(RangeFactory,
+ // Of course, we should take the constraint directly
+ // associated with this symbol into consideration.
+ getConstraint(State, Sym),
+ // Apart from the Sym itself, we can infer quite a lot if
+ // we look into subexpressions of Sym.
+ Visit(Sym));
   }
 
   RangeSet infer(EquivalenceClass Class) {
@@ -1443,38 +1450,53 @@
 return RangeFactory.deletePoint(Domain, IntType.getZeroValue());
   }
 
-  Optional getRangeForNegatedSub(SymbolRef Sym) {
+  template 
+  Optional getRangeForNegatedExpr(ProduceNegatedSymFunc F,
+QualType T) {
 // Do not negate if the type cannot be meaningfully negated.
-if (!Sym->getType()->isUnsignedIntegerOrEnumerationType() &&
-!Sym->getType()->isSignedIntegerOrEnumerationType())
+if (!T->isUnsignedIntegerOrEnumerationType() &&
+!T->isSignedIntegerOrEnumerationType())
   return llvm::None;
 
-const RangeSet *NegatedRange = nullptr;
-SymbolManager  = State->getSymbolManager();
-if (const auto *USE = dyn_cast(Sym)) {
-  if (USE->getOpcode() == UO_Minus) {
-// Just get the operand when we negate a symbol that is already negated.
-// -(-a) == a
-NegatedRange = getConstraint(State, USE->getOperand());
-  

[PATCH] D129678: [analyzer][NFC] Tidy up handler-functions in SymbolicRangeInferrer

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added a comment.

In D129678#3652859 , @ASDenysPetrov 
wrote:

> Fixed a typo that caused `constraint_manager_negate.c` and `unary-sym-expr.c` 
> tests crashes.

Good. Still LGTM.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129678/new/

https://reviews.llvm.org/D129678

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

[PATCH] D129678: [analyzer][NFC] Tidy up handler-functions in SymbolicRangeInferrer

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 444752.
ASDenysPetrov added a comment.

Fixed a typo that caused `constraint_manager_negate.c` and `unary-sym-expr.c` 
tests crashes.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129678/new/

https://reviews.llvm.org/D129678

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp

Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -1213,13 +1213,21 @@
   }
 
   RangeSet VisitSymExpr(SymbolRef Sym) {
-// If we got to this function, the actual type of the symbolic
+if (Optional RS = getRangeForNegatedSym(Sym))
+  return *RS;
+// If we've reached this line, the actual type of the symbolic
 // expression is not supported for advanced inference.
 // In this case, we simply backoff to the default "let's simply
 // infer the range from the expression's type".
 return infer(Sym->getType());
   }
 
+  RangeSet VisitUnarySymExpr(const UnarySymExpr *USE) {
+if (Optional RS = getRangeForNegatedUnarySym(USE))
+  return *RS;
+return infer(USE->getType());
+  }
+
   RangeSet VisitSymIntExpr(const SymIntExpr *Sym) {
 return VisitBinaryOperator(Sym);
   }
@@ -1228,14 +1236,25 @@
 return VisitBinaryOperator(Sym);
   }
 
-  RangeSet VisitSymSymExpr(const SymSymExpr *Sym) {
+  RangeSet VisitSymSymExpr(const SymSymExpr *SSE) {
 return intersect(
 RangeFactory,
+// If Sym is a difference of symbols A - B, then maybe we have range
+// set stored for B - A.
+//
+// If we have range set stored for both A - B and B - A then
+// calculate the effective range set by intersecting the range set
+// for A - B and the negated range set of B - A.
+getRangeForNegatedSymSym(SSE),
+// If Sym is a comparison expression (except <=>),
+// find any other comparisons with the same operands.
+// See function description.
+getRangeForComparisonSymbol(SSE),
 // If Sym is (dis)equality, we might have some information
 // on that in our equality classes data structure.
-getRangeForEqualities(Sym),
+getRangeForEqualities(SSE),
 // And we should always check what we can get from the operands.
-VisitBinaryOperator(Sym));
+VisitBinaryOperator(SSE));
   }
 
 private:
@@ -1264,25 +1283,13 @@
   }
 
   RangeSet infer(SymbolRef Sym) {
-return intersect(
-RangeFactory,
-// Of course, we should take the constraint directly associated with
-// this symbol into consideration.
-getConstraint(State, Sym),
-// If Sym is a difference of symbols A - B, then maybe we have range
-// set stored for B - A.
-//
-// If we have range set stored for both A - B and B - A then
-// calculate the effective range set by intersecting the range set
-// for A - B and the negated range set of B - A.
-getRangeForNegatedSub(Sym),
-// If Sym is a comparison expression (except <=>),
-// find any other comparisons with the same operands.
-// See function description.
-getRangeForComparisonSymbol(Sym),
-// Apart from the Sym itself, we can infer quite a lot if we look
-// into subexpressions of Sym.
-Visit(Sym));
+return intersect(RangeFactory,
+ // Of course, we should take the constraint directly
+ // associated with this symbol into consideration.
+ getConstraint(State, Sym),
+ // Apart from the Sym itself, we can infer quite a lot if
+ // we look into subexpressions of Sym.
+ Visit(Sym));
   }
 
   RangeSet infer(EquivalenceClass Class) {
@@ -1443,38 +1450,53 @@
 return RangeFactory.deletePoint(Domain, IntType.getZeroValue());
   }
 
-  Optional getRangeForNegatedSub(SymbolRef Sym) {
+  template 
+  Optional getRangeForNegatedExpr(ProduceNegatedSymFunc F,
+QualType T) {
 // Do not negate if the type cannot be meaningfully negated.
-if (!Sym->getType()->isUnsignedIntegerOrEnumerationType() &&
-!Sym->getType()->isSignedIntegerOrEnumerationType())
+if (!T->isUnsignedIntegerOrEnumerationType() &&
+!T->isSignedIntegerOrEnumerationType())
   return llvm::None;
 
-const RangeSet *NegatedRange = nullptr;
-SymbolManager  = State->getSymbolManager();
-if (const auto *USE = dyn_cast(Sym)) {
-  if (USE->getOpcode() == UO_Minus) {
-// Just get the operand when we negate a symbol that is already negated.
-// -(-a) == a
-NegatedRange = getConstraint(State, USE->getOperand());
-  }
-} else if (const SymSymExpr *SSE = dyn_cast(Sym)) {
-

[PATCH] D129678: [analyzer][NFC] Tidy up handler-functions in SymbolicRangeInferrer

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.

Took a while to accept that this is indeed an NFC, but looks good! And indeed 
the code is better structured this way.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129678/new/

https://reviews.llvm.org/D129678

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