[cfe-commits] r164339 - in /cfe/trunk: include/clang/Analysis/ include/clang/StaticAnalyzer/Core/ include/clang/StaticAnalyzer/Core/BugReporter/ include/clang/StaticAnalyzer/Core/PathSensitive/ lib/Analysis/ lib/StaticAnalyzer/Core/ test/Analysis/

[Ted Kremenek]

On Sep 20, 2012, at 7:24 PM, Jordan Rose <jordan_rose at apple.com> wrote:

>> Modified: cfe/trunk/test/Analysis/unix-fns.c
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/unix-fns.c?rev=164339&r1=164338&r2=164339&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/Analysis/unix-fns.c (original)
>> +++ cfe/trunk/test/Analysis/unix-fns.c Thu Sep 20 19:09:11 2012
>> @@ -1,4 +1,5 @@
>> -// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -analyze -analyzer-checker=unix.API,osx.API %s -analyzer-store=region -fblocks -verify
>> +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -analyze -analyzer-checker=core,unix.API,osx.API %s -analyzer-store=region -analyzer-output=plist -analyzer-ipa=inlining -analyzer-eagerly-assume -analyzer-config faux-bodies=true -fblocks -verify -o %t.plist
>> +// RUN: FileCheck --input-file=%t.plist %s 
> 
> Please add an -analyzer-output=text version of this test, and use that for -verify. I know it's imprecise, but it's easier to tell what changes when we change things.

I agree with all of your other comments, but I'm really mixed on this one.

One thing I really dislike about -analyzer-output=text is the amount of tedious work it requires to update tests.  If we refine a BugReporterVisitor, we may end up impacting a dozen tests.  That's up, except it's a pain to update them.  With the plist output, it's trivial to update tests, especially using the script update_plist_test.pl.  The plist output also directly tests the output we care about.  We can see where the control-flow edges get laid down as well as the ordering between diagnostics.  I honestly find that output really easy to read.  Yes, it's separated from the actual code, so it makes reading the diff itself a bit more cumbersome, but I really haven't found it to be a problem.

Personally, I'd really like to remove -analyzer-output=text entirely.  It doesn't accurately test the analyzer, the readability still kind of sucks, and the tests are a pain to maintain.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120921/cd1a6c02/attachment.html>