r178309 - [analyzer] Apply the suppression rules to the nil receiver only if the value participates in the computation of the nil we warn about.

[Anna Zaks]

On Mar 28, 2013, at 5:50 PM, Jordan Rose <jordan_rose at apple.com> wrote:

> I'm sad to have another one-shot visitor, rather than just doing the work up front in trackNullOrUndef.

Is there a policy on when we should walk up the path up front vs when we should do it in the visitors? Currently, we seem to be mixing both, sacrificing the cleanness of the code.. I thought that we had decided to go with visitors from now on. We could redesign that code/try to improve performance more if we think this approach is generally too slow.

> Now that suppression can take a significant portion of analysis time, we should probably not add new visitors as freely as we once would have.
> 
>> +  // The message send could be null if the receiver is null.
>> +  if (const Expr *Receiver = NilReceiverBRVisitor::getReceiver(S)) {
>> +    report.addVisitor(new NilReceiverBRVisitor(Receiver,
>> +                                               EnableNullFPSuppression));
>> +  }
>> +
> 
> if (...) {
> 	trackNullOrUndefValue(N, Receiver, report);
> }
> 

The receiver might not be nil. Logically, we should only register to track it if it is indeed nil.  We cannot know if it is nil or not until we walk up the path to find the node where it has a value. Using the existing visitor to do this seems to be the cleanest approach. That was the motivation for doing it this way.

> This seems much more efficient—no restarting of path generation due to the new visitor.



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130329/fea894f9/attachment.html>