On Mar 20, 2012, at 17:06, Anna Zaks wrote:

> On Mar 20, 2012, at 4:51 PM, Jordan Rose wrote:
> 
>> On Mar 20, 2012, at 16:30, Anna Zaks wrote:
>> 
>>> This seems to implement the first suggestion by Ted, where we attempt to 
>>> regenerate a single PathDiagnosticPiece, not the whole path, which would 
>>> not work for state-full visitors. Am I missing something?
>> 
>> Oh, I wasn't sure that was the final decision. It makes things a little 
>> easier on the checker writer to worry about visiting the same nodes twice, 
>> but MallocChecker is certainly still stateful. Still, if we really don't 
>> care about performance during diagnostic generation...
>> 
> 
> It's not the matter of performance, nor our decision will only influence the 
> Malloc checker. (Note Malloc visitor is not the only one keeping internal 
> state.)
> 
> This is the checker API redesign issue. Any checker writer should be able to 
> add interesting symbols without looking at the diagnostics code and making 
> sure the way they track the state in the checker visitor is compliant with 
> it. The checker writer should not worry about visiting the same node twice, 
> they don't even know it's happening. 

Fair enough. I'll go rework this. Advance question: is it better to wait until 
the whole path is generated before restarting, or to restart as soon as we see 
the interesting symbols have changed? Both ways could result in a second round 
of new interesting symbols.


>>> On Mar 20, 2012, at 4:20 PM, Jordan Rose wrote:
>>> 
>>>> Any suggestions for alternate names for 
>>>> getInterestingObjectSetDescriptor()? I'm trying to make it clear it's an 
>>>> opaque value that's only supposed to be used for detecting changes...
>>> 
>>> I don't think visitors should be implementing a callback for this. You 
>>> should be able to just check if the interesting symbols set in the bug 
>>> reporter changed.. 
>> 
>> Oh, it's not a visitor callback -- the interesting symbols and regions are 
>> stored in the BugReport. So it's just communication between the BugReport 
>> and the BugReporter.
> 
> Sorry, You are right.  I am a bit concerned with this method. It assumes that 
> the symbols are never deleted from the set. Is this currently true?

Yes, it is. If that changes, the implementation of this method will of course 
have to change. Is it worth proactively adding a change counter to BugReport?


_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to