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

[John McCall]

On Aug 20, 2012, at 10:38 AM, Anna Zaks wrote:
> On Aug 19, 2012, at 2:27 PM, John McCall wrote:
>> On Aug 17, 2012, at 7:59 PM, Jordan Rose wrote:
>>> 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.)
>> 
>> FWIW, I generally find myself wanting to use binary enums for most of
>> these things eventually.  That would be nicer if clang were using C++11,
>> of course.
>> 
> 
> Using the enum or multiple methods is a better solution than the comment. Also, LLVM Coding Standard makes no exception for using C style comments for dealing with cryptic arguments:
> http://llvm.org/docs/CodingStandards.html#comment-formatting

That's definitely just a missing case in the coding standard;  this is a widely-used idiom, even if I do generally feel that there are better alternatives.

John.

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