[PATCH][RFC] llvm-cov HTML generation

Mon Mar 14 11:11:04 PDT 2016

I haven’t been able to reproduce this issue. Not quite sure why you’re seeing it.

If you could help me isolate why that might happen, I’d appreciate it.

In the meantime, a small final patch to silence that switch warning and avoid some unnecessary copies.

Thanks,
Harlan

> On Mar 11, 2016, at 1:14 AM, Ying Yi <maggieyi666 at gmail.com> wrote:
> 
> 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 <mailto:hhaskins at apple.com>> wrote:
> 
>> On Mar 10, 2016, at 1:30 PM, Ying Yi <maggieyi666 at gmail.com <mailto: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 <mailto: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 <mailto:mail at justinbogner.com>> wrote:
>>> 
>>> Harlan Haskins <hhaskins at apple.com <mailto:hhaskins at apple.com>> writes:
>>>> Thanks again! Responses inline.
>>>>> On Mar 8, 2016, at 5:08 AM, Ying Yi <maggieyi666 at gmail.com <mailto: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/20160314/3fc07bd3/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: llvm-cov-html.diff
Type: application/octet-stream
Size: 56075 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160314/3fc07bd3/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160314/3fc07bd3/attachment-0001.html>