[PATCH][RFC] llvm-cov HTML generation

[Ying Yi via llvm-commits]

These are the steps that I used to get the issue:

1) Build:
   $ cmake -G "Unix Makefiles"
-DCMAKE_INSTALL_PREFIX=prefix=~/upstream/bin  -DCMAKE_BUILD_TYPE=Release
-DLLVM_ENABLE_ASSERTIONS=On ~/upstream/llvm
   $ make

2) Test:
   $ cat test.cpp
   int main(int argc, char ** argv) {
      int x = 1;

      for (int i = 0; i < 1000; ++i)
          x *= 2+1;

      return x;
   }

  $ ~/upstream/build/bin/clang -O0 -fprofile-instr-generate=test.profraw
-fcoverage-mapping "test.cpp"
  $ ./a.out
  $ ~/upstream/build/bin/llvm-profdata merge test.profraw -o merged.profdata
  $ ~/upstream/build/bin/llvm-cov show -format=html -output-dir=./report
a.out -instr-profile=merged.profdata


Thanks,



On Thu, Mar 10, 2016 at 10:41 PM, Harlan Haskins <hhaskins at apple.com> wrote:

>
> On Mar 10, 2016, at 1:30 PM, Ying Yi <maggieyi666 at gmail.com> wrote:
>
> Thanks for your reply and the updated patch.
>
> 1) When I use the release build of llvm-cov to generate HTML files, all
> the HTML files are named “html”. This issue also exists on UNIX.
>
> I haven’t seen this happen. Can you provide some steps to reproduce?
> Thanks!
>
> 2) The following warning is given when building llvm-cov using Visual
> Studio 2013.
>
> “sourcecoverageview.cpp (108): warning C4715:
> 'llvm::SourceCoverageView::create' : not all control paths return a value”
>
> That warning is incorrect. It’s a switch statement over both cases of an
> enum — I want this because if we add another format, say, OFJSON or
> something, I want a compiler warning telling me the switch case isn’t
> exhaustive.
>
> Here’s an updated patch with some small sanity tests added:
>
>
>
> Best,
> Harlan
>
>
> On Wed, Mar 9, 2016 at 12:50 AM, Harlan Haskins <hhaskins at apple.com>
> wrote:
>
>> Updated patch with unistd removed, utf-8, and `console` replaced with
>> `text`.
>>
>> Thanks!
>> Harlan
>>
>>
>> On Mar 8, 2016, at 10:55 AM, Justin Bogner <mail at justinbogner.com> wrote:
>>
>> Harlan Haskins <hhaskins at apple.com> writes:
>>
>> Thanks again! Responses inline.
>>
>> On Mar 8, 2016, at 5:08 AM, Ying Yi <maggieyi666 at gmail.com> wrote:
>>
>> Thanks for adding the index page. I have some comments:
>>
>> 1) The ‘unistd.h’ header won’t build on windows.
>>
>> Thanks for catching that — I was using it to debug, but don’t need
>> it. I also don’t have a Windows machine to debug, so thank you for
>> investigating portability!
>>
>> 2) The help text for ‘-format’ is different from llvm-profdata.exe
>> tool, which uses the following style:
>>    -format=<html|console>                 - Format to output comparison ….
>>    Is there a standard way to do this?
>>
>> LLVM’s CommandLine library renders the help text. Also, I noticed I
>> used ‘comparison’ instead of ‘coverage’. Thanks!
>>
>> 3) Should ‘console’ be changed to ‘text’, because it doesn’t show in
>> the console when you give an output directory?
>>
>> Yeah, I think it could. I’ve cc’d Justin for his input — he originally
>> suggested Console.
>>
>>
>> My only problem with "text" is that this includes ANSI escape sequences
>> when it uses colour, so text seems a bit misleading. That said, it
>> doesn't matter much - I'm fine with either.
>>
>> 4) Could you please use an html charset that supports UTF-8?
>>
>> Yep. Thanks for catching that!
>>
>> 5) The HTML report can look quite different to the source file if it
>> includes tabs. Could you specify a smaller tab-size in the CSS file?
>>
>> That’s a good question, and I don’t quite know the correct answer to
>> it. GitHub has a complicated set of rules for tab sizing that makes a
>> ‘best guess’ based on the source file type. I wonder if it’s a good
>> candidate for a command line switch?
>>
>> In the future (not for this patch):
>>
>> 6) Could we support a user CSS file?
>> Currently, the CSS rules are embedded into the html page. Could
>> llvm-cov provide an option to use an external CSS file? This would
>> give the user more flexibility.
>>
>> I agree that it’d give the user more flexibility, but it’d be
>> difficult to do given the nature of the directory structure. Maybe, if
>> you know you’re gonna be running it on a server where the ‘root’ is
>> well-defined, we could just dump the css file to disk and put a
>> relative <link> tag in. But if it’s gonna be ten directories down,
>> we’ll need to have some logic in that replaces all the parent paths in
>> the link with /../..’s to make the file resolve. I think it can be
>> done, just with more input from the user about their intention with
>> hosting.
>>
>> 7) Could we put the functions/lines summary information into the
>> HTML files? As a rough example:
>>
>> Lines: 9/10 (90%)
>> Functions: 1/1 (100%)
>>
>> I think this totally can be done — it’s out of scope for this patch
>> (it’s already a pretty big patch as it is.)
>> Some kind of merging of ‘show’ and ‘report’ would be ideal. Maybe the
>> index page has at-a-glance information about each file?
>>
>>
>> Yep, this sounds like a great idea, but it's probably more suited to a
>> follow up patch.
>>
>>
>>
>>
>
>
> --
> Ying Yi
> SN Systems Ltd - Sony Computer Entertainment Group.
>
>
>
>


-- 
Ying Yi
SN Systems Ltd - Sony Computer Entertainment Group.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160311/d1d9d770/attachment-0001.html>