Alexander_Droste 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; }; ---------------- dcoughlin wrote: > 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. Yes, the string includes the name of the struct. But this is not due to the while loop in this function but how `printPretty` is defined for `FieldRegion`. In case of a `FieldRegion` `printPretty` is called for the super-region first before the field declation name gets appended. ``` void FieldRegion::printPrettyAsExpr(raw_ostream &os) const { assert(canPrintPrettyAsExpr()); superRegion->printPrettyAsExpr(os); os << "." << getDecl()->getName(); } ``` I'll go with `getDescriptiveName`.
http://reviews.llvm.org/D16044 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
