[PATCH] D115883: [analyzer][NFC] Change return value of StoreManager::attemptDownCast function from SVal to Optional

[Denys Petrov via Phabricator via cfe-commits]

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

Refactor return value of `StoreManager::attemptDownCast` function by removing 
the last parameter `bool &Failed` and replace the return value `SVal` with 
`Optional<SVal>`.  Make the function consistent with the family of 
`evalDerivedToBase` by renaming it to `evalBaseToDerived`. Aligned the code on 
the call side with these changes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115883

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/lib/StaticAnalyzer/Core/Store.cpp


Index: clang/lib/StaticAnalyzer/Core/Store.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/Store.cpp
+++ clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -314,10 +314,7 @@
   return nullptr;
 }
 
-SVal StoreManager::attemptDownCast(SVal Base, QualType TargetType,
-                                   bool &Failed) {
-  Failed = false;
-
+Optional<SVal> StoreManager::evalBaseToDerived(SVal Base, QualType TargetType) {
   const MemRegion *MR = Base.getAsRegion();
   if (!MR)
     return UnknownVal();
@@ -392,7 +389,9 @@
   }
 
   // We failed if the region we ended up with has perfect type info.
-  Failed = isa<TypedValueRegion>(MR);
+  if (isa<TypedValueRegion>(MR))
+    return None;
+
   return UnknownVal();
 }
 
Index: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -439,14 +439,15 @@
         if (CastE->isGLValue())
           resultType = getContext().getPointerType(resultType);
 
-        bool Failed = false;
-
-        // Check if the value being cast evaluates to 0.
-        if (val.isZeroConstant())
-          Failed = true;
-        // Else, evaluate the cast.
-        else
-          val = getStoreManager().attemptDownCast(val, T, Failed);
+        bool Failed = true;
+
+        // Check if the value being cast does not evaluates to 0.
+        if (!val.isZeroConstant())
+          if (Optional<SVal> V =
+                  StateMgr.getStoreManager().evalBaseToDerived(val, T)) {
+            val = *V;
+            Failed = false;
+          }
 
         if (Failed) {
           if (T->isReferenceType()) {
@@ -478,14 +479,13 @@
         if (CastE->isGLValue())
           resultType = getContext().getPointerType(resultType);
 
-        bool Failed = false;
-
         if (!val.isConstant()) {
-          val = getStoreManager().attemptDownCast(val, T, Failed);
+          Optional<SVal> V = getStoreManager().evalBaseToDerived(val, T);
+          val = V ? *V : UnknownVal();
         }
 
         // Failed to cast or the result is unknown, fall back to conservative.
-        if (Failed || val.isUnknown()) {
+        if (val.isUnknown()) {
           val =
             svalBuilder.conjureSymbolVal(nullptr, CastE, LCtx, resultType,
                                          currBldrCtx->blockCount());
Index: clang/lib/StaticAnalyzer/Core/CallEvent.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -762,9 +762,9 @@
       QualType Ty = Ctx.getPointerType(Ctx.getRecordType(Class));
 
       // FIXME: CallEvent maybe shouldn't be directly accessing StoreManager.
-      bool Failed;
-      ThisVal = StateMgr.getStoreManager().attemptDownCast(ThisVal, Ty, Failed);
-      if (Failed) {
+      Optional<SVal> V =
+          StateMgr.getStoreManager().evalBaseToDerived(ThisVal, Ty);
+      if (!V.hasValue()) {
         // We might have suffered some sort of placement new earlier, so
         // we're constructing in a completely unexpected storage.
         // Fall back to a generic pointer cast for this-value.
@@ -772,7 +772,8 @@
         const CXXRecordDecl *StaticClass = StaticMD->getParent();
         QualType StaticTy = Ctx.getPointerType(Ctx.getRecordType(StaticClass));
         ThisVal = SVB.evalCast(ThisVal, Ty, StaticTy);
-      }
+      } else
+        ThisVal = *V;
     }
 
     if (!ThisVal.isUnknown())
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
@@ -172,9 +172,9 @@
   ///    dynamic_cast.
   ///  - We don't know (base is a symbolic region and we don't have
   ///    enough info to determine if the cast will succeed at run time).
-  /// The function returns an SVal representing the derived class; it's
-  /// valid only if Failed flag is set to false.
-  SVal attemptDownCast(SVal Base, QualType DerivedPtrType, bool &Failed);
+  /// The function returns an optional with SVal representing the derived class
+  /// in case of a successful cast and `None` otherwise.
+  Optional<SVal> evalBaseToDerived(SVal Base, QualType DerivedPtrType);
 
   const ElementRegion *GetElementZeroRegion(const SubRegion *R, QualType T);
 

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