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:
> k-wisniewski wrote:
> > NoQ wrote:
> > > 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.
> > How about the situation when this callback is triggered by something other
> > than a call, like variable assignment? I've tried using that location
> > context and had lots of segfaults as in such cases it appeared to be null.
> > Do you have some info that it should be non-null in such a case?
> Ok, makes sense. Have you looked into providing a checker context there? How
> much more difficult would that be? If that is too difficult, adding
> LocationContext is good as well.
>
> If Call is optional and LocationContext is not, should the Call parameter be
> last.
If we add a CheckerContext, then we may end up calling callbacks that split
states within callbacks that split states, and that'd be quite a mess to
support.
Right now a checker may trigger `checkRegionChanges()` within its callback by
manipulating the Store, which already leads to callbacks within callbacks, but
that's bearable as long as you can't add transitions within the inner callbacks.
https://reviews.llvm.org/D26588
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
