[PATCH] D55697: [analyzer] Assume that we always have a SubEngine available

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC349266: [analyzer] Assume that we always have a SubEngine 
available (authored by xazax, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55697?vs=178204=178357#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D55697

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
  lib/StaticAnalyzer/Core/CallEvent.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ProgramState.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp

Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -454,7 +454,7 @@
   QualType SingleTy;
 
   auto  =
-StateMgr.getOwningEngine()->getAnalysisManager().getAnalyzerOptions();
+StateMgr.getOwningEngine().getAnalysisManager().getAnalyzerOptions();
 
   // FIXME: After putting complexity threshold to the symbols we can always
   //rearrange additive operations but rearrange comparisons only if
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -550,15 +550,15 @@
 return RuntimeDefinition(Decl);
   }
 
-  SubEngine *Engine = getState()->getStateManager().getOwningEngine();
-  AnalyzerOptions  = Engine->getAnalysisManager().options;
+  SubEngine  = getState()->getStateManager().getOwningEngine();
+  AnalyzerOptions  = Engine.getAnalysisManager().options;
 
   // Try to get CTU definition only if CTUDir is provided.
   if (!Opts.IsNaiveCTUEnabled)
 return {};
 
   cross_tu::CrossTranslationUnitContext  =
-  *Engine->getCrossTranslationUnitContext();
+  *Engine.getCrossTranslationUnitContext();
   llvm::Expected CTUDeclOrError =
   CTUCtx.getCrossTUDefinition(FD, Opts.CTUDir, Opts.CTUIndexName,
   Opts.DisplayCTUProgress);
@@ -1087,7 +1087,7 @@
  Selector Sel) const {
   assert(IDecl);
   AnalysisManager  =
-  getState()->getStateManager().getOwningEngine()->getAnalysisManager();
+  getState()->getStateManager().getOwningEngine().getAnalysisManager();
   // If the class interface is declared inside the main file, assume it is not
   // subcassed.
   // TODO: It could actually be subclassed if the subclass is private as well.
Index: lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -384,7 +384,7 @@
   // TODO: When the Max Complexity is reached, we should conjure a symbol
   // instead of generating an Unknown value and propagate the taint info to it.
   const unsigned MaxComp = StateMgr.getOwningEngine()
-   ->getAnalysisManager()
+   .getAnalysisManager()
.options.MaxSymbolComplexity;
 
   if (symLHS && symRHS &&
Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -125,8 +125,8 @@
   ProgramStateRef newState = makeWithStore(Mgr.StoreMgr->Bind(getStore(),
  LV, V));
   const MemRegion *MR = LV.getAsRegion();
-  if (MR && Mgr.getOwningEngine() && notifyChanges)
-return Mgr.getOwningEngine()->processRegionChange(newState, MR, LCtx);
+  if (MR && notifyChanges)
+return Mgr.getOwningEngine().processRegionChange(newState, MR, LCtx);
 
   return newState;
 }
@@ -138,9 +138,7 @@
   const MemRegion *R = loc.castAs().getRegion();
   const StoreRef  = Mgr.StoreMgr->BindDefaultInitial(getStore(), R, V);
   ProgramStateRef new_state = makeWithStore(newStore);
-  return Mgr.getOwningEngine()
- ? Mgr.getOwningEngine()->processRegionChange(new_state, R, LCtx)
- : new_state;
+  return Mgr.getOwningEngine().processRegionChange(new_state, R, LCtx);
 }
 
 ProgramStateRef
@@ -149,9 +147,7 @@
   const MemRegion *R = loc.castAs().getRegion();
   const StoreRef  = Mgr.StoreMgr->BindDefaultZero(getStore(), R);
   ProgramStateRef new_state = makeWithStore(newStore);
-  return Mgr.getOwningEngine()
- ? Mgr.getOwningEngine()->processRegionChange(new_state, R, LCtx)
- : new_state;
+  return Mgr.getOwningEngine().processRegionChange(new_state, R, LCtx);
 }
 
 typedef ArrayRef RegionList;
@@ -196,41 +192,34 @@
   

[PATCH] D55697: [analyzer] Assume that we always have a SubEngine available

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D55697#1331001 , @Szelethus wrote:

> We could also add an assert to getOwningEngine I guess.


Or make it return a reference, like everything else.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55697



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

[PATCH] D55697: [analyzer] Assume that we always have a SubEngine available

[Artem Dergachev via Phabricator via cfe-commits]

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

Yup, squash these! Even if there was a plan to use `ProgramState` separately 
for some other sort of analysis that doesn't involve a sub-engine, it is 
currently long forgotten.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55697



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

[PATCH] D55697: [analyzer] Assume that we always have a SubEngine available

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

We could also add an assert to getOwningEngine I guess.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55697



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

[PATCH] D55697: [analyzer] Assume that we always have a SubEngine available

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun created this revision.
xazax.hun added reviewers: NoQ, george.karpenkov.
xazax.hun added a project: clang.
Herald added subscribers: gamesh411, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity.

I was stumbled upon some checks whether a SubEngine is available. I was 
wondering why do we have those checks so I removed them and wondered if the 
tests would break. They did not break on my machine, so I submitted this patch.

Do yo think these should be removed? If no, how could we test those code paths 
because apparently they are untested at the moment.


Repository:
  rC Clang

https://reviews.llvm.org/D55697

Files:
  lib/StaticAnalyzer/Core/ProgramState.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp

Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -347,11 +347,10 @@
 : StoreManager(mgr), Features(f),
   RBFactory(mgr.getAllocator()), CBFactory(mgr.getAllocator()),
   SmallStructLimit(0) {
-if (SubEngine *Eng = StateMgr.getOwningEngine()) {
-  AnalyzerOptions  = Eng->getAnalysisManager().options;
-  SmallStructLimit =
-Options.RegionStoreSmallStructLimit;
-}
+SubEngine *Eng = StateMgr.getOwningEngine();
+AnalyzerOptions  = Eng->getAnalysisManager().options;
+SmallStructLimit =
+  Options.RegionStoreSmallStructLimit;
   }
 
 
Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -125,7 +125,7 @@
   ProgramStateRef newState = makeWithStore(Mgr.StoreMgr->Bind(getStore(),
  LV, V));
   const MemRegion *MR = LV.getAsRegion();
-  if (MR && Mgr.getOwningEngine() && notifyChanges)
+  if (MR && notifyChanges)
 return Mgr.getOwningEngine()->processRegionChange(newState, MR, LCtx);
 
   return newState;
@@ -138,9 +138,7 @@
   const MemRegion *R = loc.castAs().getRegion();
   const StoreRef  = Mgr.StoreMgr->BindDefaultInitial(getStore(), R, V);
   ProgramStateRef new_state = makeWithStore(newStore);
-  return Mgr.getOwningEngine()
- ? Mgr.getOwningEngine()->processRegionChange(new_state, R, LCtx)
- : new_state;
+  return Mgr.getOwningEngine()->processRegionChange(new_state, R, LCtx);
 }
 
 ProgramStateRef
@@ -149,9 +147,7 @@
   const MemRegion *R = loc.castAs().getRegion();
   const StoreRef  = Mgr.StoreMgr->BindDefaultZero(getStore(), R);
   ProgramStateRef new_state = makeWithStore(newStore);
-  return Mgr.getOwningEngine()
- ? Mgr.getOwningEngine()->processRegionChange(new_state, R, LCtx)
- : new_state;
+  return Mgr.getOwningEngine()->processRegionChange(new_state, R, LCtx);
 }
 
 typedef ArrayRef RegionList;
@@ -198,39 +194,32 @@
   ProgramStateManager  = getStateManager();
   SubEngine* Eng = Mgr.getOwningEngine();
 
-  InvalidatedSymbols Invalidated;
+  InvalidatedSymbols InvalidatedSyms;
   if (!IS)
-IS = 
+IS = 
 
   RegionAndSymbolInvalidationTraits ITraitsLocal;
   if (!ITraits)
 ITraits = 
 
-  if (Eng) {
-StoreManager::InvalidatedRegions TopLevelInvalidated;
-StoreManager::InvalidatedRegions Invalidated;
-const StoreRef 
-= Mgr.StoreMgr->invalidateRegions(getStore(), Values, E, Count, LCtx, Call,
-  *IS, *ITraits, ,
-  );
-
-ProgramStateRef newState = makeWithStore(newStore);
-
-if (CausedByPointerEscape) {
-  newState = Eng->notifyCheckersOfPointerEscape(newState, IS,
-TopLevelInvalidated,
-Call,
-*ITraits);
-}
+  StoreManager::InvalidatedRegions TopLevelInvalidated;
+  StoreManager::InvalidatedRegions Invalidated;
+  const StoreRef 
+  = Mgr.StoreMgr->invalidateRegions(getStore(), Values, E, Count, LCtx, Call,
+*IS, *ITraits, ,
+);
+
+  ProgramStateRef newState = makeWithStore(newStore);
 
-return Eng->processRegionChanges(newState, IS, TopLevelInvalidated,
- Invalidated, LCtx, Call);
+  if (CausedByPointerEscape) {
+newState = Eng->notifyCheckersOfPointerEscape(newState, IS,
+  TopLevelInvalidated,
+  Call,
+  *ITraits);
   }
 
-  const StoreRef  =
-  Mgr.StoreMgr->invalidateRegions(getStore(), Values, E, Count, LCtx, Call,
-  *IS, *ITraits, nullptr, nullptr);
-  return makeWithStore(newStore);
+  return