[PATCH] D26588: Add LocationContext to members of check::RegionChanges

[Aleksei Sidorin via cfe-commits]

a.sidorin added a comment.

Hi Krzysztof!
This change  seems useful: I can imagine the situation where we want to ask current `LocationContext` in this callback. The change looks pretty intrusive but I mostly agree with it. Initially, I have some questions about the implementation of `getArgSVal()` function (inline comments). I'll add more comments later.



================
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:735
+  const FunctionDecl *FunctionDecl = SFC->getDecl()->getAsFunction();
+  unsigned NumArgs = FunctionDecl->getNumParams();
+  assert(ArgIdx < NumArgs && "Arg access out of range!");
----------------
Maybe we should put a check that requested StackFrame is our StackFrame or our parent StackFrame here?


================
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:741
+    // because the call wasn't modeled in the first place.
+    const VarDecl *ArgDecl = FunctionDecl->parameters()[ArgIdx];
+    const Loc ArgLoc = getLValue(ArgDecl, SFC);
----------------
Unfortunately, this code does not consider the fact that argument values may be overwritten. If we want to get initial values, we should find another way.


https://reviews.llvm.org/D26588