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

Aleksei Sidorin via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 5 00:50:42 PST 2016


a.sidorin added inline comments.

================
Comment at: llvm/tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:1186
@@ +1185,3 @@
+  ///  associated with a specified globally stored, non-static local, VarDecl.
+  const MemRegion *getMemRegionGloballyStored(const VarDecl *D);
+
----------------
ariccio wrote:
> a.sidorin wrote:
> > How about make these helper functions return `const MemSpaceRegion *` to make their signatures more meaningful?
> Would that change their behavior functionally?
No, it will not change it. You will need to change `static_cast` type to `const MemSpaceRegion *`, however (lines 809-810). There is `const MemSpaceRegion *` and its children classes everywhere in return statements already.

================
Comment at: llvm/tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp:843
@@ +842,3 @@
+                                                const LocationContext *LC) {
+  const MemRegion *sReg = nullptr;
+
----------------
ariccio wrote:
> a.sidorin wrote:
> > `const MemSpaceRegion *StorageSpace?`
> Same question as above: Would that change their behavior functionally?
> 
> 
> (if not, then I'll happily change it)
No, this will not.


http://reviews.llvm.org/D16873





More information about the cfe-commits mailing list