[cfe-commits] r162156 - in /cfe/trunk: lib/StaticAnalyzer/Core/ExprEngine.cpp test/Analysis/exceptions.mm

[Jordan Rose]

On Aug 17, 2012, at 19:15 , Anna Zaks <ganna at apple.com> wrote:

> 
> On Aug 17, 2012, at 5:30 PM, Jordan Rose wrote:
> 
>> Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=162156&r1=162155&r2=162156&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
>> +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Fri Aug 17 19:30:20 2012
>> @@ -889,7 +889,7 @@
>>    case Stmt::ObjCAtThrowStmtClass: {
>>      // FIXME: This is not complete.  We basically treat @throw as
>>      // an abort.
>> -      Bldr.generateNode(S, Pred, Pred->getState());
>> +      Bldr.generateNode(S, Pred, Pred->getState(), /*IsSink=*/true);
> Please remove the C style comment.
> You can mention that we are generating a sink in a comment or better yet add another API to the node builder spelling out what we do:
> "Bldr.generateSink"

This is a common way to annotate otherwise cryptic parameters in Clang, though admittedly most of the uses in the analyzer specifically were introduced by me. I don't understand why it's an issue, at least right now. (I agree that in this case a "generateSink" method would be a little clearer.)

In the longer term, as you suggested offline, C++ 'throw' may be best modeled by having more than one kind of sink node. I actually started down this path, at which point the third parameter here became the node type (Node, Sink, or Jump -- my name for "program continues but analysis cannot"). That wasn't actually sufficient to model C++ 'throw', however, and so I opted for the simplest and least-intrusive solution.

(...but that's a mild argument against introducing a new method now; having more than two kinds of nodes means possibly adding another extra method in every place we can generate nodes, rather than just changing the third parameter.)

Jordan