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

[Anna Zaks]

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

Talked to Jordan off line and we agreed to change the API and keep the analyzer codebase free of C Style comments.
Anna.

> John.

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