dcoughlin added inline comments.
================
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:160
@@ -153,1 +159,3 @@
+ /// \returns variable name for memory region
+ std::string getVariableName() const;
};
----------------
Alexander_Droste wrote:
> dcoughlin wrote:
> > 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()'?
> > The returned name is not always the name of a variable.
> In which cases doesn't this function return the name? Isn't the worst case
> when `printPretty` can't be called and the string is empty?
>
> The function is intended to return the variable name and collects index
> variable names / `ConcreteInt`s along the way if the passed region is an
> `ElementRegion`. So `printPretty` is called on the 'top' region of an array
> at the end of this function. If the passed region is no `ElementRegion` the
> functionality is identical to `printPretty`.
>
> The main motivation to write this function was to include specific indices in
> a diagnostic if obtainable, like presented in the MPI-Checker testfile
> http://reviews.llvm.org/D12761#3c51dfcf.
> E.g. `Request 'sendReq1[1][7][9]' has no matching nonblocking call.` My
> intent was not to use the function for debugging purposes.
>
> So should we use `getDescriptiveName` or maybe sth. like
> `getVariableNameWithIndices`?.
Can the string ever be something like 'a.f[0][1]' (i.e., with a field name)? Or
is it always 'a[0][1]'? If the first is possible I would suggest
`getDescriptiveName` -- if not, then `getVariableNameWithIndices` seems fine.
================
Comment at: lib/StaticAnalyzer/Core/MemRegion.cpp:635
@@ -634,1 +634,3 @@
+std::string MemRegion::getVariableName() const {
+ std::string VariableName;
----------------
Alexander_Droste wrote:
> dcoughlin wrote:
> > 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?
> Modifying `printPretty` seems like a nice solution to me. If you don't prefer
> the other idea, I would go this route.
Sounds good to me!
http://reviews.llvm.org/D16044
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
