[PATCH] D126123: [analyzer][NFC] MemRegion::getRegion() never returns null

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D126123#3531150 , @steakhal wrote:

> In D126123#3531112 , @martong wrote:
>
>> Is it documented with `getRegion`?  Could we decorate that with 
>> `returns-nonnull` 
>> https://clang.llvm.org/docs/AttributeReference.html#returns-nonnull ?
>
> Ah, I thought one of my changes addressing this was already on review.
> It turns out it was not the case. Now I uploaded it for review: D126198 
> 

Ok, could you please change the title of this Differential Revision? Something 
is still not fitting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126123

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

[PATCH] D126123: [analyzer][NFC] MemRegion::getRegion() never returns null

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

In D126123#3531112 , @martong wrote:

> Is it documented with `getRegion`?  Could we decorate that with 
> `returns-nonnull` 
> https://clang.llvm.org/docs/AttributeReference.html#returns-nonnull ?

Ah, I thought one of my changes addressing this was already on review.
It turns out it was not the case. Now I uploaded it for review: D126198 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126123

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

[PATCH] D126123: [analyzer][NFC] MemRegion::getRegion() never returns null

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Is it documented with `getRegion`?  Could we decorate that with 
`returns-nonnull` 
https://clang.llvm.org/docs/AttributeReference.html#returns-nonnull ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126123

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

[PATCH] D126123: [analyzer][NFC] MemRegion::getRegion() never returns null

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

Until this point, the runtime and reports of the Clang Static Analyzer did not 
change on our testset; which confirms that the stack is indeed NFC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126123

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

[PATCH] D126123: [analyzer][NFC] MemRegion::getRegion() never returns null

[Balázs Benics via Phabricator via cfe-commits]

steakhal updated this revision to Diff 431168.
steakhal added reviewers: NoQ, martong.
steakhal added a comment.
Herald added a subscriber: rnkovacs.

Well, my first use of arc went a bit sideways, so I'm falling back to posting 
this NFC patch batch manually.
We will see how using arc works out next time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126123

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


Index: clang/lib/StaticAnalyzer/Core/SVals.cpp
===
--- clang/lib/StaticAnalyzer/Core/SVals.cpp
+++ clang/lib/StaticAnalyzer/Core/SVals.cpp
@@ -177,8 +177,7 @@
 }
 
 const MemRegion *loc::MemRegionVal::stripCasts(bool StripBaseCasts) const {
-  const MemRegion *R = getRegion();
-  return R ?  R->StripCasts(StripBaseCasts) : nullptr;
+  return getRegion()->StripCasts(StripBaseCasts);
 }
 
 const void *nonloc::LazyCompoundVal::getStore() const {


Index: clang/lib/StaticAnalyzer/Core/SVals.cpp
===
--- clang/lib/StaticAnalyzer/Core/SVals.cpp
+++ clang/lib/StaticAnalyzer/Core/SVals.cpp
@@ -177,8 +177,7 @@
 }
 
 const MemRegion *loc::MemRegionVal::stripCasts(bool StripBaseCasts) const {
-  const MemRegion *R = getRegion();
-  return R ?  R->StripCasts(StripBaseCasts) : nullptr;
+  return getRegion()->StripCasts(StripBaseCasts);
 }
 
 const void *nonloc::LazyCompoundVal::getStore() const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126123: [analyzer][NFC] MemRegion::getRegion() never returns null

[Balázs Benics via Phabricator via cfe-commits]

steakhal updated this revision to Diff 431155.
steakhal added a comment.

`arc` used the HEAD^ as base, so I'm setting the parent patches now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126123

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
  clang/lib/StaticAnalyzer/Core/SVals.cpp
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp

Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -56,8 +56,6 @@
   : SValBuilder(alloc, context, stateMgr) {}
   ~SimpleSValBuilder() override {}
 
-  SVal evalMinus(NonLoc val) override;
-  SVal evalComplement(NonLoc val) override;
   SVal evalBinOpNN(ProgramStateRef state, BinaryOperator::Opcode op,
NonLoc lhs, NonLoc rhs, QualType resultTy) override;
   SVal evalBinOpLL(ProgramStateRef state, BinaryOperator::Opcode op,
@@ -82,28 +80,6 @@
   return new SimpleSValBuilder(alloc, context, stateMgr);
 }
 
-//===--===//
-// Transfer function for unary operators.
-//===--===//
-
-SVal SimpleSValBuilder::evalMinus(NonLoc val) {
-  switch (val.getSubKind()) {
-  case nonloc::ConcreteIntKind:
-return val.castAs().evalMinus(*this);
-  default:
-return UnknownVal();
-  }
-}
-
-SVal SimpleSValBuilder::evalComplement(NonLoc X) {
-  switch (X.getSubKind()) {
-  case nonloc::ConcreteIntKind:
-return X.castAs().evalComplement(*this);
-  default:
-return UnknownVal();
-  }
-}
-
 // Checks if the negation the value and flipping sign preserve
 // the semantics on the operation in the resultType
 static bool isNegationValuePreserving(const llvm::APSInt ,
@@ -845,6 +821,8 @@
 return UnknownVal();
 
   case loc::ConcreteIntKind: {
+auto L = lhs.castAs();
+
 // If one of the operands is a symbol and the other is a constant,
 // build an expression for use by the constraint manager.
 if (SymbolRef rSym = rhs.getAsLocSymbol()) {
@@ -853,19 +831,17 @@
   if (!BinaryOperator::isComparisonOp(op) || op == BO_Cmp)
 return UnknownVal();
 
-  const llvm::APSInt  = lhs.castAs().getValue();
   op = BinaryOperator::reverseComparisonOp(op);
-  return makeNonLoc(rSym, op, lVal, resultTy);
+  return makeNonLoc(rSym, op, L.getValue(), resultTy);
 }
 
 // If both operands are constants, just perform the operation.
 if (Optional rInt = rhs.getAs()) {
-  SVal ResultVal =
-  lhs.castAs().evalBinOp(BasicVals, op, *rInt);
-  if (Optional Result = ResultVal.getAs())
-return evalCast(*Result, resultTy, QualType{});
+  assert(BinaryOperator::isComparisonOp(op) || op == BO_Sub);
 
-  assert(!ResultVal.getAs() && "Loc-Loc ops should not produce Locs");
+  if (const auto *ResultInt =
+  BasicVals.evalAPSInt(op, L.getValue(), rInt->getValue()))
+return evalCast(nonloc::ConcreteInt(*ResultInt), resultTy, QualType{});
   return UnknownVal();
 }
 
Index: clang/lib/StaticAnalyzer/Core/SVals.cpp
===
--- clang/lib/StaticAnalyzer/Core/SVals.cpp
+++ clang/lib/StaticAnalyzer/Core/SVals.cpp
@@ -43,25 +43,6 @@
 // Utility methods.
 //===--===//
 
-bool SVal::hasConjuredSymbol() const {
-  if (Optional SV = getAs()) {
-SymbolRef sym = SV->getSymbol();
-if (isa(sym))
-  return true;
-  }
-
-  if (Optional RV = getAs()) {
-const MemRegion *R = RV->getRegion();
-if (const auto *SR = dyn_cast(R)) {
-  SymbolRef sym = SR->getSymbol();
-  if (isa(sym))
-return true;
-}
-  }
-
-  return false;
-}
-
 const FunctionDecl *SVal::getAsFunctionDecl() const {
   if (Optional X = getAs()) {
 const MemRegion* R = X->getRegion();
@@ -196,8 +177,7 @@
 }
 
 const MemRegion *loc::MemRegionVal::stripCasts(bool StripBaseCasts) const {
-  const MemRegion *R = getRegion();
-  return R ?  R->StripCasts(StripBaseCasts) : nullptr;
+  return getRegion()->StripCasts(StripBaseCasts);
 }
 
 const void *nonloc::LazyCompoundVal::getStore() const {
@@ -272,49 +252,6 @@
   return isConstant(0);
 }
 
-//===--===//
-// Transfer function dispatch for Non-Locs.
-//===--===//
-
-SVal nonloc::ConcreteInt::evalBinOp(SValBuilder ,
- 

[PATCH] D126123: [analyzer][NFC] MemRegion::getRegion() never returns null

[Balázs Benics via Phabricator via cfe-commits]

steakhal created this revision.
Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp, donat.nagy, 
Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a project: All.
steakhal requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126123

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


Index: clang/lib/StaticAnalyzer/Core/SVals.cpp
===
--- clang/lib/StaticAnalyzer/Core/SVals.cpp
+++ clang/lib/StaticAnalyzer/Core/SVals.cpp
@@ -177,8 +177,7 @@
 }
 
 const MemRegion *loc::MemRegionVal::stripCasts(bool StripBaseCasts) const {
-  const MemRegion *R = getRegion();
-  return R ?  R->StripCasts(StripBaseCasts) : nullptr;
+  return getRegion()->StripCasts(StripBaseCasts);
 }
 
 const void *nonloc::LazyCompoundVal::getStore() const {


Index: clang/lib/StaticAnalyzer/Core/SVals.cpp
===
--- clang/lib/StaticAnalyzer/Core/SVals.cpp
+++ clang/lib/StaticAnalyzer/Core/SVals.cpp
@@ -177,8 +177,7 @@
 }
 
 const MemRegion *loc::MemRegionVal::stripCasts(bool StripBaseCasts) const {
-  const MemRegion *R = getRegion();
-  return R ?  R->StripCasts(StripBaseCasts) : nullptr;
+  return getRegion()->StripCasts(StripBaseCasts);
 }
 
 const void *nonloc::LazyCompoundVal::getStore() const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits