[PATCH][RFC] llvm-cov HTML generation

Tue Mar 8 10:59:25 PST 2016

On Tue, Mar 8, 2016 at 10:25 AM, Harlan Haskins <hhaskins at apple.com> wrote:

> 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.
>
> 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?
>

A command line switch is reasonable as a followup, I think.

David

> 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?
>
> Many thanks,
>
>
> Maggie
>
>
>
> On Tue, Mar 8, 2016 at 12:56 AM, Harlan Haskins <hhaskins at apple.com>
> wrote:
>
>> I really need to get better at patches.
>>
>> Updated patch without the build directory:
>>
>>
>>
>> On Mar 7, 2016, at 2:03 PM, Harlan Haskins via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>> Thanks! I’m still working out how to properly tests the HTML generation
>> (I need to build a compatible version of clang to make compatible profdata
>> inputs.)
>>
>> In the meantime, I think I’ve fixed the portability issues that you and
>> Maggie suggested, as well as a preliminary index implementation and a
>> better design.
>>
>> Let me know what you think!
>>
>> Thanks,
>> Harlan
>>
>> <llvm-cov-html.diff>
>> <macro-coverage.zip>
>>
>> On Mar 7, 2016, at 1:26 PM, Xinliang David Li <xinliangli at gmail.com>
>> wrote:
>>
>> The patch looks pretty good now.
>>
>> >    std::string OutputPath = OutputDirectory;
>> >    if (OutputPath != "") {
>> >      sys::fs::create_directories(OutputDirectory);
>> >      OutputPath += "/functions." + FileExt;
>>
>> This is not portable. Try:
>>
>>   llvm::sys::path::append(...) method
>>
>> There are other places with similar code.
>>
>> And it seems test cases are still missing?
>>
>> thanks,
>>
>> David
>>
>>
>>
>>
>>
>>
>> On Fri, Mar 4, 2016 at 10:46 AM, Harlan Haskins via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>> Somehow the CSS include file didn’t make it into that diff.
>>>
>>> — Harlan
>>>
>>> On Mar 3, 2016, at 5:02 PM, Harlan Haskins via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>>
>>> Hi all,
>>>
>>> Thanks again for the reviews!
>>>
>>> I’ve restructured how I handle subviews and pulled out the common
>>> behavior. I also addressed the issue with highlighting and showing macro
>>> expansions (and found a possible bug in clang because of it).
>>>
>>> Attached is a) a new patch, and b) an HTML file showing a single line
>>> multi-macro expansion.
>>>
>>> Thanks,
>>> Harlan Haskins
>>>
>>> <macro.c.html>
>>> <llvm-cov-html.diff>
>>>
>>> On Mar 2, 2016, at 3:00 PM, Harlan Haskins via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>>
>>> Oh, I see! Yeah, this seems like something I overlooked. The HTML view
>>> currently just shows two expansions, one after the other.
>>>
>>> I’ll revise the subview rendering with this in mind. Thanks for the
>>> simple example case!
>>>
>>> Best,
>>> Harlan
>>>
>>> On Mar 2, 2016, at 2:40 PM, Xinliang David Li via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>>
>>>
>>>
>>> On Wed, Mar 2, 2016 at 1:41 PM, Harlan Haskins <hhaskins at apple.com>
>>> wrote:
>>>
>>>> Hi David,
>>>>
>>>> Specifically with renderSubviews, in my refactor it seemed that their
>>>> bodies were different enough (i.e. SourceCoverageViewConsole needs to track
>>>> state between loop invocations to know whether or not to display a final
>>>> view divider, and the common behavior is really just looping over the
>>>> instantiation and expansion subviews. I can investigate converging them
>>>> more, but I think it’s going to increase complexity.
>>>>
>>>
>>> Emitting the final divider is easy to abstract away. The main difference
>>> I see is that in Console view, if there are multiple macro expansions in
>>> the same line, the line will be re-rendered again in order to highlight the
>>> macro. For instance given the following line with two macros,
>>>
>>>    MY_MACRO(10, 10); MY_MACRO(20,10);    // line 10
>>>
>>> The console dump will be two lines:
>>>
>>> 1|   10| MY_MACRO(10, 10); MY_MACRO(20,10);    // line 10
>>>          ^^^^^^^^^^^
>>>       .... expansion lines
>>>
>>>          MY_MACRO(10, 10); MY_MACRO(20,10);    // line 10
>>>                                                        ^^^^^^^^^^^^^
>>>         .... expansion lines
>>>
>>>
>>> Does HTML view lose that functionality?
>>>
>>> Also the template instantiation subview rendering code looks
>>> sufficiently close between two classes.
>>>
>>> thanks,
>>>
>>> David
>>>
>>>
>>>
>>>>
>>>> Also, I definitely need to add a test case.
>>>>
>>>> Thanks!
>>>> Harlan
>>>>
>>>> On Mar 2, 2016, at 10:43 AM, Xinliang David Li <xinliangli at gmail.com>
>>>> wrote:
>>>>
>>>> Hi Harlan,
>>>>
>>>> This looks great! Some high level comments. I find the code can be
>>>> further restructured
>>>> 1) high level methods can be commoned between two derived classes  (and
>>>> pushed to the base class) -- such as renderSubviews
>>>> 2) the subclasses just need to provide virtual functions that implement
>>>> the view specific low level routines.
>>>>
>>>> Also there does not seem to be a test case.
>>>>
>>>> thanks,
>>>>
>>>> David
>>>>
>>>> On Tue, Mar 1, 2016 at 6:03 PM, Harlan Haskins via llvm-commits <
>>>> llvm-commits at lists.llvm.org> wrote:
>>>>
>>>>> Oops, forgot to add a file to the patch.
>>>>>
>>>>> New patch attached.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> On Mar 1, 2016, at 5:48 PM, Harlan Haskins via llvm-commits <
>>>>> llvm-commits at lists.llvm.org> wrote:
>>>>>
>>>>> Hi all,
>>>>>
>>>>> I’ve got a preliminary implementation of HTML generation for
>>>>> llvm-cov’s coverage reports.
>>>>>
>>>>> The patch adds 2 flags to llvm-cov show:
>>>>>
>>>>>    - -format [html | console]
>>>>>    - -output-dir <dirname>
>>>>>
>>>>>
>>>>> Specifying -format=console will perform the current behavior (now the
>>>>> default), and -format=html will generate an HTML report.
>>>>> If -output-dir is specified, then the output is split into one html or
>>>>> txt file per source file, named <source-name>.<ext>, with a directory
>>>>> structure that mimics the file system structure.
>>>>>
>>>>> If neither are provided, the behavior remains the same.
>>>>>
>>>>> I’m hoping to add an index with a browsable list of files within their
>>>>> directories, but for now I’ve attached the patch and a sample HTML file (In
>>>>> this case, AliasAnalysis.h, as included by swiftc).
>>>>>
>>>>> Thanks,
>>>>> Harlan Haskins
>>>>>
>>>>> <AliasAnalysis.h.html>
>>>>> <llvm-cov-html.diff>
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>>
>>>>>
>>>>
>>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>>
>>
>>
>
>
> --
> 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/20160308/8037b49a/attachment.html>