<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""></div><div class=""><br class=""></div><div class="">Thanks,</div><div class="">Harlan</div><div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Mar 11, 2016, at 1:14 AM, Ying Yi <<a href="mailto:maggieyi666@gmail.com" class="">maggieyi666@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">These are the steps that I used to get the issue:<br class=""><br class="">1) Build:<br class=""> $ cmake -G "Unix Makefiles" -DCMAKE_INSTALL_PREFIX=prefix=~/upstream/bin -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=On ~/upstream/llvm<br class=""> $ make<br class=""><br class="">2) Test:<br class=""> $ cat test.cpp<br class=""> int main(int argc, char ** argv) {
<br class=""> int x = 1;
<br class=""> <br class=""> for (int i = 0; i < 1000; ++i)
<br class=""> x *= 2+1;
<br class=""> <br class=""> return x;
<br class=""> }<br class=""><br class=""> $ ~/upstream/build/bin/clang -O0 -fprofile-instr-generate=test.profraw -fcoverage-mapping "test.cpp"<br class=""> $ ./a.out<br class=""> $ ~/upstream/build/bin/llvm-profdata merge test.profraw -o merged.profdata<br class=""> $ ~/upstream/build/bin/llvm-cov show -format=html -output-dir=./report a.out -instr-profile=merged.profdata<br class=""><br class=""><br class="">Thanks,<br class=""><br class=""><br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Thu, Mar 10, 2016 at 10:41 PM, Harlan Haskins <span dir="ltr" class=""><<a href="mailto:hhaskins@apple.com" target="_blank" class="">hhaskins@apple.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word" class=""><br class=""><div class=""><span class=""><blockquote type="cite" class=""><div class="">On Mar 10, 2016, at 1:30 PM, Ying Yi <<a href="mailto:maggieyi666@gmail.com" target="_blank" class="">maggieyi666@gmail.com</a>> wrote:</div><br class=""><div class=""><div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><p class="MsoNormal"><span style="font-size:10pt;line-height:15.3333px" class="">Thanks for your reply and the updated patch.</span></p></div></div></blockquote><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><p class="MsoNormal"><span style="font-size:10pt;line-height:15.3333px" class="">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.</span></p></div></div></blockquote></span>I haven’t seen this happen. Can you provide some steps to reproduce? Thanks!<span class=""><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><p class="MsoNormal"><span style="font-size:10pt;line-height:15.3333px" class="">2) The following warning is given when building llvm-cov using Visual Studio 2013.</span></p><p class="MsoNormal"><span style="font-size:10pt;line-height:15.3333px;color:rgb(30,30,30);background-image:none;background-color:white;background-repeat:repeat repeat" class="">“sourcecoverageview.cpp (108): warning C4715: 'llvm::SourceCoverageView::create' : not all control paths return a value</span><span style="font-size:10pt;line-height:15.3333px;color:rgb(30,30,30)" class="">”</span></p></div></div></blockquote></span>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.</div><div class=""><br class=""></div><div class="">Here’s an updated patch with some small sanity tests added:</div><div class=""><br class=""></div><div class=""></div></div><br class=""><div style="word-wrap:break-word" class=""><div class=""></div><div class=""><br class=""></div><div class="">Best,</div><div class="">Harlan</div><div class=""><br class=""><blockquote type="cite" class=""><div class="gmail_extra" style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><br class=""><div class="gmail_quote">On Wed, Mar 9, 2016 at 12:50 AM, Harlan Haskins<span class=""> </span><span dir="ltr" class=""><<a href="mailto:hhaskins@apple.com" target="_blank" class="">hhaskins@apple.com</a>></span><span class=""> </span>wrote:<br class=""><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word" class="">Updated patch with unistd removed, utf-8, and `console` replaced with `text`.<div class=""><br class=""></div><div class="">Thanks!</div><span class=""><font color="#888888" class=""><div class="">Harlan<br class=""><div class=""></div></div></font></span></div><br class=""><div style="word-wrap:break-word" class=""><div class=""><div class=""></div><div class=""><br class=""><div class=""><blockquote type="cite" class=""><div class="">On Mar 8, 2016, at 10:55 AM, Justin Bogner <<a href="mailto:mail@justinbogner.com" target="_blank" class="">mail@justinbogner.com</a>> wrote:</div><br class=""><div class=""><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important" class="">Harlan Haskins <</span><a href="mailto:hhaskins@apple.com" style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" target="_blank" class="">hhaskins@apple.com</a><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important" class="">> writes:</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><blockquote type="cite" style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class="">Thanks again! Responses inline.<br class=""><blockquote type="cite" class="">On Mar 8, 2016, at 5:08 AM, Ying Yi <<a href="mailto:maggieyi666@gmail.com" target="_blank" class="">maggieyi666@gmail.com</a>> wrote:<br class=""><br class="">Thanks for adding the index page. I have some comments:<br class=""><br class="">1) The ‘unistd.h’ header won’t build on windows.<br class=""><br class=""></blockquote>Thanks for catching that — I was using it to debug, but don’t need<br class="">it. I also don’t have a Windows machine to debug, so thank you for<br class="">investigating portability!<br class=""><br class=""><blockquote type="cite" class="">2) The help text for ‘-format’ is different from llvm-profdata.exe<br class="">tool, which uses the following style:<br class=""> -format=<html|console> - Format to output comparison ….<br class=""> Is there a standard way to do this?<br class=""><br class=""></blockquote>LLVM’s CommandLine library renders the help text. Also, I noticed I<br class="">used ‘comparison’ instead of ‘coverage’. Thanks!<br class=""><br class=""><blockquote type="cite" class="">3) Should ‘console’ be changed to ‘text’, because it doesn’t show in<br class="">the console when you give an output directory?<br class=""><br class=""></blockquote>Yeah, I think it could. I’ve cc’d Justin for his input — he originally<br class="">suggested Console.<br class=""></blockquote><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important" class="">My only problem with "text" is that this includes ANSI escape sequences</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important" class="">when it uses colour, so text seems a bit misleading. That said, it</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important" class="">doesn't matter much - I'm fine with either.</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><blockquote type="cite" style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><blockquote type="cite" class="">4) Could you please use an html charset that supports UTF-8?<br class=""><br class=""></blockquote>Yep. Thanks for catching that!<br class=""><br class=""><blockquote type="cite" class="">5) The HTML report can look quite different to the source file if it<br class="">includes tabs. Could you specify a smaller tab-size in the CSS file?<br class=""><br class=""></blockquote>That’s a good question, and I don’t quite know the correct answer to<br class="">it. GitHub has a complicated set of rules for tab sizing that makes a<br class="">‘best guess’ based on the source file type. I wonder if it’s a good<br class="">candidate for a command line switch?<br class=""><br class=""><blockquote type="cite" class="">In the future (not for this patch):<span class=""> </span><br class=""><br class="">6) Could we support a user CSS file?<span class=""> </span><br class="">Currently, the CSS rules are embedded into the html page. Could<br class="">llvm-cov provide an option to use an external CSS file? This would<br class="">give the user more flexibility.<br class=""><br class=""></blockquote>I agree that it’d give the user more flexibility, but it’d be<br class="">difficult to do given the nature of the directory structure. Maybe, if<br class="">you know you’re gonna be running it on a server where the ‘root’ is<br class="">well-defined, we could just dump the css file to disk and put a<br class="">relative <link> tag in. But if it’s gonna be ten directories down,<br class="">we’ll need to have some logic in that replaces all the parent paths in<br class="">the link with /../..’s to make the file resolve. I think it can be<br class="">done, just with more input from the user about their intention with<br class="">hosting.<br class=""><br class=""><blockquote type="cite" class="">7) Could we put the functions/lines summary information into the<br class="">HTML files? As a rough example:<br class=""><br class="">Lines: 9/10 (90%)<br class="">Functions: 1/1 (100%)<br class=""><br class=""></blockquote>I think this totally can be done — it’s out of scope for this patch<br class="">(it’s already a pretty big patch as it is.)<br class="">Some kind of merging of ‘show’ and ‘report’ would be ideal. Maybe the<br class="">index page has at-a-glance information about each file?<br class=""></blockquote><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important" class="">Yep, this sounds like a great idea, but it's probably more suited to a</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important" class="">follow up patch.</span></div></blockquote></div><br class=""></div></div></div><br class=""></blockquote></div><br class=""><br clear="all" class=""><br class="">--<span class=""> </span><br class=""><div class=""><div dir="ltr" class=""><div class=""><div dir="ltr" class=""><div class=""><div dir="ltr" class=""><font size="2" class=""><span style="font-family:arial,helvetica,sans-serif" class="">Ying Yi</span><span style="font-family:arial,helvetica,sans-serif" class=""><br class="">SN Systems Ltd - Sony Computer Entertainment Group.</span></font></div></div></div></div></div></div></div></blockquote></div><br class=""></div><br class=""></blockquote></div><br class=""><br clear="all" class=""><br class="">-- <br class=""><div class="gmail_signature"><div dir="ltr" class=""><div class=""><div dir="ltr" class=""><div class=""><div dir="ltr" class=""><font size="2" class=""><span style="font-family:arial,helvetica,sans-serif" class="">Ying Yi</span><span style="font-family:arial,helvetica,sans-serif" class=""><br class="">SN Systems Ltd - Sony Computer Entertainment Group.</span><span style="font-family:arial,helvetica,sans-serif" class=""></span></font><br class=""></div></div></div></div></div></div>
</div></div>
</div></blockquote></div><br class=""></div></body></html>