[PATCH] D16044: getVariableName() for MemRegion

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

xazax.hun added inline comments.

================
Comment at: tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp:660
@@ +659,3 @@
+    }
+
+    else if (auto SV =
----------------
The else should go into the same line as the closing }.

================
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;
----------------
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?

================
Comment at: tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp:669
@@ +668,3 @@
+
+    ArrayIndices += "]" + idx + "[";
+    R = ER->getSuperRegion();
----------------
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.

================
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);
----------------
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. 


http://reviews.llvm.org/D16044