zaks.anna added a comment.

> Hmm, i'm thinking of just the opposite - refactor the CallEvent methods to 
> use the respective new  ProgramState methods.
> 
> This way we'd avoid touching the CallEvent allocator for a simple Environment 
> lookup, while still avoiding code duplication.

I see what you are suggesting, but I still have the concerns I've mentioned in 
my first comment. **The whole purpose of CallEvent is to allow an easy and 
lightweight way of dealing with all the different types of calls**, which is 
exactly what we are doing in this checker. CallEvent is used for this exact 
purpose elsewhere throughout the analyzer. What do you mean by "touching  the 
CallEvent allocator" and why would you want to discourage that?

CallEventManager::getCaller contains logic to dispatch to different CallEvent 
types depending on what call we are dealing with. The argument lookup will 
depend on type of the call. The method added in this PR does not deal with all 
call types, but only with CallExpr, so you'd need to duplicate the code from 
CallEvent construction to here.

As I've mentioned before, I do not think that Call dispatch logic and API for 
looking up arguments SVals should belong to ProgramState as it is a higher 
abstraction and should not reason about LocationContexts or Calls (which are 
part of Program Location and AST respectively). In addition, using CallEven to 
look up argument values is much simpler to understand for API users.


https://reviews.llvm.org/D27091



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

Reply via email to