[PATCH][RFC] llvm-cov HTML generation

Ying Yi via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 8 05:08:24 PST 2016


Thanks for adding the index page. I have some comments:



1) The ‘unistd.h’ header won’t build on windows.



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?



3) Should ‘console’ be changed to ‘text’, because it doesn’t show in the
console when you give an output directory?



4) Could you please use an html charset that supports UTF-8?



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?



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.



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%)





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/4da271bf/attachment.html>


More information about the llvm-commits mailing list