[PATCH] D16044: getVariableName() for MemRegion

[Alexander Droste via cfe-commits]

Alexander_Droste added inline comments.

================
Comment at: tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp:662
@@ +661,3 @@
+    else if (auto SV =
+        ER->getIndex().getAs<clang::ento::nonloc::SymbolVal>()) {
+        llvm::SmallString<8> buf;
----------------
xazax.hun wrote:
> These are the only cases that can occur? So it is not possible to have a loc index? Maybe we could get the value for loc and nonloc ConcreteInts and use getVariableName for everzthing else? Would that wok?
`getIndex()` returns a `nonloc` and `loc` is in no subclass relation to `nonloc`.
Therefore, `getIndex()` cannot be a `loc` type.

There are actually 5 `nonloc` subclasses but I assumed 
that it only makes sense to check for `SymbolVal` and 
`ConcreteInt`. 
http://clang.llvm.org/doxygen/classclang_1_1ento_1_1NonLoc.html

So would you suggest to only check for `ConcreteInt`
and call `getVariableName()` recursively if it isn't one?

================
Comment at: tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp:669
@@ +668,3 @@
+
+    ArrayIndices += "]" + idx + "[";
+    R = ER->getSuperRegion();
----------------
xazax.hun wrote:
> In this case idx also needs to be reserved. Since it is very rare that there are a huge level of []s, it is possible that the Twine solution is more desirable.
Ok, I'll look into the Twine API.

================
Comment at: tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp:682
@@ +681,3 @@
+  // Combine variable name with indices.
+  if (VariableName.size() && VariableName.back() == '\'') {
+    VariableName.insert(VariableName.size() - 1, ArrayIndices);
----------------
xazax.hun wrote:
> What is happening here exactly? In some cases there are ' symbols around the name and in other cases there aren't? Is there a way to solve this inconsistency (so we do not need to check for the last character)? Also it would be great that this functions always had consistent result, so either never put ' around the name or always. 
I think I wasn't completely sure if single quotes
are always included when `printPretty()` is called.
I looked into `MemRegion.cpp` and they always seem
to be included, why the second case can be removed.


http://reviews.llvm.org/D16044