[PATCH] D16873: Refactor MemRegionManager::getVarRegion to call two new functions, improving readability

[Alexander Riccio via cfe-commits]

ariccio added a comment.

One of two comments addressed, one question asked.


================
Comment at: llvm/tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp:792
@@ +791,3 @@
+
+const MemRegion* MemRegionManager::getMemSpaceForLocalVariable(const VarDecl *D, llvm::PointerUnion<const StackFrameContext *, const VarRegion *> &V) {
+  const StackFrameContext *STC = V.get<const StackFrameContext*>();
----------------
a.sidorin wrote:
> That needs to be formatted. Also, you may pass `const StackFrameContext *` instead of `PointerUnion` (as Devin pointed) because it is only used once to get SFC.
> And I still think that making these function return `const MemSpaceRegion *` is a good idea.
If I change it to a `MemSpaceRegion*`, do I need to change the `static_cast`s, from:
```
           ? static_cast<const MemRegion*>(getStackArgumentsRegion(STC))
           : static_cast<const MemRegion*>(getStackLocalsRegion(STC)));

```

to:
```
           ? static_cast<const MemSpaceRegion*>(getStackArgumentsRegion(STC))
           : static_cast<const MemSpaceRegion*>(getStackLocalsRegion(STC)));

```
...?

I'm still not quite sure what they're for in the first place?


http://reviews.llvm.org/D16873