[PATCH] D16044: getVariableName() for MemRegion

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

Hi Alexander,

Some comments in line. Also, I don't see any tests. Is this code tested by your MPI patch?


================
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:160
@@ -153,1 +159,3 @@
+  /// \returns variable name for memory region
+  std::string getVariableName() const;
 };
----------------
I'm not sure that 'getVariableName()' conveys what this function does. The returned name is not always the name of a variable. How do you intend to use this method? How is it different than than printPretty()? Is it for debugging purposes? If so, then perhaps 'getDebugDescription()'? Is it for presentation to the user? Then perhaps 'getDescriptiveName()'?

================
Comment at: lib/StaticAnalyzer/Core/MemRegion.cpp:635
@@ -634,1 +634,3 @@
 
+std::string MemRegion::getVariableName() const {
+  std::string VariableName;
----------------
It seems like this could potentially be simplified by adding a recursive helper that threads the output stream and appends to it after the base case has been hit. You could also modify prettyPrint() to take a flag indicating whether it should be quoted or not (and defaulting to true). What do you think?


http://reviews.llvm.org/D16044