[PATCH] D18278: llvm-cov HTML Generation

Justin Bogner via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 6 12:08:26 PDT 2016


Vedant Kumar <vsk at apple.com> writes:
> vsk added a comment.
>
> In http://reviews.llvm.org/D18278#473437, @MaggieYi wrote:
>
>> There are two issues with the current patch on Windows, I am happy
>> to patch these later if you want.
>>
>> When a project has two files with the same file name but in
>> different drives (e.g. C:\temp\temp.c and D:\temp\temp.c), the
>> llvm-cov.exe tool will only show one of these files. This could be
>> fixed by using the drive letter (or network drive name) as a part of
>> the file output path.
>>
>> The source file link will not work if the full file path exceeds the
>> maximum path length (260 characters). Could we give a warning in
>> this case?
>
>
> Thanks for testing it out!
>
> It seems that all the pending issues we are discussing actually relate
> to `CoveragePrinter::getOutputPath`, which is not a part of this
> patch. We can fix them before or after this patch is committed, and
> test the changes using "-format text -output-dir ...".
>
> Given that this is a large change, I'd like to make sure it addresses
> Justin's concerns and that it has an explicit lgtm before moving
> forward.

I didn't look in too much detail, as the other reviewers seem to have it
covered, but the structure and concept LGTM. Thanks for working on this!


More information about the llvm-commits mailing list