NoQ added inline comments.

================
Comment at: include/clang/StaticAnalyzer/Core/Checker.h:325
+                      const CallEvent *Call,
+                      const LocationContext *LCtx) {
+    return ((const CHECKER *) checker)->checkRegionChanges(state, invalidated,
----------------
zaks.anna wrote:
> LocationContext can be obtained by calling CallEvent::getLocationContext(). I 
> do not think that adding another parameter here buys us much. Am I missing 
> something?
CallEvent* is optional here - it's there only for invalidations through calls.


================
Comment at: include/clang/StaticAnalyzer/Core/Checker.h:333
+                                       ProgramStateRef state,
+                                       const LocationContext *LCtx) {
+    return ((const CHECKER *) checker)->wantsRegionChangeUpdate(state, LCtx);
----------------
zaks.anna wrote:
> What is the scenario in which you need the context here? The idea is that the 
> checker would be interested in region changes if it is already tracking 
> information in the state. For that check, the information in the state is 
> sufficient. What is your scenario?
> 
> Also, For any checker API change, we need to update the 
> CheckerDocumentation.cpp.
Environment, which is a significant portion of the state, is essentially 
unaccessible without a location context.

Call stack is also an important bit of information to any checker that does 
something special to support/exploit the inlining IPA - there are many checkers 
that scan the call stack, but so far none of them needed to subscribe to 
checkRegionChanges; their possible use case may look like "we want an update 
only if a certain condition was met within the current stack frame".

For the recursion checker, i think, the point is "if current frame is spoiled, 
we no longer want updates" (which should probably be fixed in the checker 
patch: even though the callback is broken, it'd take one less action to fix it 
if we decide to).


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