On May 12, 2013, at 7:04 PM, Sam Handler <[email protected]> wrote:
> > > > On Mon, May 6, 2013 at 8:43 PM, Jordan Rose <[email protected]> wrote: >> +If needed, the <tt>addTransition</tt> function can be called with no >> arguments to return the current >> +<tt>ExplodedNode</tt> without changing the program's state. > > This isn't exactly true; addTransition returns a new node without changing > the current node's state, but it does return a new node, either for emitting > a bug report or just putting your stamp on whatever you are checking. If you > use addTransition just to get the current node, it's all to you end up > splitting the path. > > Calling addTransition without arguments does not in fact appear to return a > new node. > > ExplodedNode *addTransition(ProgramStateRef State = 0, > const ProgramPointTag *Tag = 0) { > return addTransitionImpl(State ? State : getState(), false, 0, Tag); > } > > ExplodedNode *addTransitionImpl(ProgramStateRef State, > bool MarkAsSink, > ExplodedNode *P = 0, > const ProgramPointTag *Tag = 0) { > if (!State || (State == Pred->getState() && !Tag && !MarkAsSink)) > return Pred; > ... > > > Since getState() returns Pred->getState(), calling addTransition() is the > same as calling addTransitionImpl(Pred->getState(), false, 0, 0), which will > simply return Pred. > Sam, Thanks for drawing the attention to this. I think Jordan is right, the checker should tag the node when using it for a BugReport. Otherwise, the analyzer engine's internal node reclamation might remove the node altogether (at least in theory). I vaguely recall adding the if statement as a micro optimization. If it was not there, addTransition() would create a new tagged node. I'll try taking out the if statement and see if it results in any performance regressions. Thanks, Anna. > I'm not sure if this behavior is intentional, although there are many > existing checkers that call addTransition with no arguments to get the > ExplodedNode for a bug report. > > In any case, I've changed the text to recommend getPredecessor(), as this > seems like the more logical choice. I've also made it clearer that this can > only be used if no state transition has been performed. > > All other comments should be addressed. > > --Sam > > > -- > =============================================================================== > All opinions expressed in posts from this account are entirely my own, and not > those of any present or former employer. Furthermore, I assert that all works > contributed to the Clang project (1) were developed using no equipment, > supplies, facility or trade secrets of any such employer, (2) were developed > entirely on my own time, and (3) do not result from any work performed for any > such employer. > > <checker_dev_manual.diff>
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
