<div dir="ltr"><div dir="ltr"><div dir="ltr">@reid, Currently, the GCNO/GCDA format are the same as in gcc 4.2 (<a href="https://llvm.org/docs/CommandGuide/llvm-cov.html">https://llvm.org/docs/CommandGuide/llvm-cov.html</a>)</div><div>So adding extra data will break the compatibility with gcc 4.2.</div><div><br></div><div>@vedant, an example of what we've currently with a code similar to yours:</div><div>-: 0:Source:d.c<br>-: 0:Graph:d.gcno<br>-: 0:Data:d.gcda<br>-: 0:Runs:1<br>-: 0:Programs:1<br>-: 1:int foo(int x) {<br>3: 2: if (x == 0) {<br>1: 3: return 1;<br>2: 4: } else if (x == 1) {<br>1: 5: return 2;<br>-: 6: }<br>-: 7:<br>1: 8: return 3;<br>3: 9:}<br>-: 10:<br>-: 11:int main(int argc, char ** argv) {<br>1: 12: foo(0);<br>1: 13: foo(1);<br>1: 14: foo(2);<br>-: 15:<br>1: 16: return 3;<br>-: 17:}</div><div><br></div><div>I'm not sure that the 3 on line 9 is so useful because finally it relies on how it's implemented behind (in the IR we only have one final ret instruction).</div>From my point of view, a code coverage report should be easily understandable (I mean no need to do maths) and not rely on compiler details.<br></div><div dir="ltr"><div>I agree with you on the fact that having two modes is painful and from my point of view when I made this patch (reverted) in clang, I had the impress to fix a bug and not to add a feature.</div><div>So what could we do to go ahead ?</div><div><br> </div></div></div><br><div class="gmail_quote"><div dir="ltr">Le lun. 1 oct. 2018 à 23:14, Reid Kleckner <<a href="mailto:rnk@google.com">rnk@google.com</a>> a écrit :<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I have no understanding of any of the file formats involved, but it seems like it would be the kind of thing we'd record in the information about the source location, and then the tool that generates text or HTML can display it how it likes at the end. That's why I support putting the info in the LLVM IR format, since we'll need to track the bit through instrumentation.<div><br></div><div>If the file format is too hard to extend, then I wouldn't mind adding an extra option for people who want to tweak our instrumentation.</div></div><br><div class="gmail_quote"><div dir="ltr">On Wed, Sep 26, 2018 at 3:10 PM Vedant Kumar <<a href="mailto:vsk@apple.com" target="_blank">vsk@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space">Hi,<br><div><br><blockquote type="cite"><div>On Sep 25, 2018, at 12:46 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:</div><br class="m_1363720289889087503m_-3293631296756800476Apple-interchange-newline"><div><div dir="ltr">(+ a few folks who've been involved in the reviews, etc)<br><br><div class="gmail_quote"><div dir="ltr">On Mon, Sep 24, 2018 at 12:34 PM Calixte Denizet via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div>Hi,</div><div><br></div><div>When making code coverage, there are some counters on closing braces.</div><div>In some case it's due to "return void" with a debugloc corresponding to '}' (see line 4) or due to cleanup stuff put at the end of a scope (see line 20, aaa's dtor is called).</div><div>Especially with c++, some "implicit" code (i.e. not written explicitly by the user) could be added at the beginning of a scope and at the end.<br></div><div>Here are two example of outputs produced by clang and gcc.</div><div><br></div><div>With clang 7:<br>-: 1:struct A {<br>-: 2:<br>1: 3: A() {}<br>-: 4: <br>-: 5: ~A() {<br>1: 6: }<br>-: 7:};<br>-: 8:<br>-: 9:void foo(int K) {<br>1: 10:}<br>-: 11:<br>-: 12:int main() {<br>1: 13: A aaa;<br>1: 14: int x = 1;<br>1: 15: foo(x);<br>1: 16: x = x + 2;<br>-: 17:<br>1: 18: return x;<br>1: 19:}<br></div><br><div>With gcc 8:<br>-: 1:struct A {<br>-: 2:<br>1: 3: A() {}<br>-: 4: <br>1: 5: ~A() {<br>1: 6: }<br>-: 7:};<br>-: 8:<br>1: 9:void foo(int K) {<br>1: 10:}<br>-: 11:<br>1: 12:int main() {<br>1: 13: A aaa;<br>1: 14: int x = 1;<br>1: 15: foo(x);<br>1: 16: x = x + 2;<br>-: 17:<br>1: 18: return x;<br>-: 19:}</div><div><br></div><div>So I'd like to have coverage for lines which contain only "explicit" code to have a clear information to give to different kinds of people (I spent myself some time to understand why I had coverage on line like "} // namespace foo”).</div></div></div></div></div></div></blockquote></div></div></div></blockquote><div><br></div>Makes sense, although I’ll note that there are cases where hiding coverage stats on closing braces might be doing a disservice, like:</div><div><br></div><div>void foo() {</div><div> if (…)</div><div> return;</div><div> if (…)</div><div> return;</div><div>} //< The coverage here might be interesting if you care about how often you don’t return early.</div><div><br></div><div>You can figure it out by subtracting the coverage counts on return statements from the function entry count, but that’s work the coverage tool could do for you.</div><div><br><blockquote type="cite"><div><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div>So we could add an option in clang (no idea of the name right now) to allow user to have coverage for explicit code.</div></div></div></div></div></div></blockquote></div></div></div></blockquote><div><br></div><div>I’d rather not have a flag to control this behavior. I imagine that most users of a coverage tool will never change its default behavior. From a maintenance perspective, it’s easier to test one coverage mode than two.</div><div><br></div><div>You’re making a compelling argument that hiding coverage stats on implicit code is the right thing to do (if for no other reason than “it’s what gcc does”). If most others agree with you, I’d rather have such a change made unconditionally even though it’s not my personal preference.</div><div><br></div><div>Just my 2 cents — happy to defer to others maintaining/using gcov.</div><div><br></div><div>thanks,</div><div>vedant</div><div><br></div><br><blockquote type="cite"><div><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div>And so have this output:<br>-: 1:struct A {<br>-: 2:<br>1: 3: A() {}<br>-: 4: <br>1: 5: ~A() {<br>-: 6: }<br>-: 7:};<br>-: 8:<br>1: 9:void foo(int K) {<br>-: 10:}<br>-: 11:<br>1: 12:int main() {<br>1: 13: A aaa;<br>1: 14: int x = 1;<br>1: 15: foo(x);<br>1: 16: x = x + 2;<br>-: 17:<br>1: 18: return x;<br>-: 19:}<br></div><div><br></div><div>Calixte<br></div></div></div></div></div></div>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div></div>
</div></blockquote></div><br></div></blockquote></div>
</blockquote></div>