NoQ added a comment.

Welcome to phabricator! I agree that having the location context in this 
callback is useful, and i'm all for reducing boilerplate in various checkers 
through better API.



================
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:231
 
   ProgramStateRef bindLoc(Loc location,
                           SVal V,
----------------
Because this API becomes more complicated, i think we should add some 
docstrings here, explaining the meaning of the location context parameter, in 
particular.


================
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:733
+inline SVal ProgramState::getArgSVal(const StackFrameContext *SFC,
+                              const unsigned ArgIdx) const {
+  const FunctionDecl *FunctionDecl = SFC->getDecl()->getAsFunction();
----------------
Indent.


================
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);
----------------
a.sidorin wrote:
> 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.
Uhm, yeah, this is in fact a problem!

The purpose of this code is, in fact, to construct `SymbolRegionValue` for the 
parameter, so it is equivalent to calling 
`SValBuilder::getRegionValueSymbolVal()` over the parameter region, as long as 
the current `StoreManager` implementation remains unquestioned.

We could also ask `StoreManager` to provide a binding for this region from an 
empty store, maybe extend its API to allow such queries, this would remove the 
layering violation.

Because this topic getting is rather complicated, it might have been better to 
make a separate review for this change, originally (no problem now that most of 
the interested people have already had a look).


https://reviews.llvm.org/D26588



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to