<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Tue, Sep 5, 2017 at 10:22 AM Paul Robinson via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">probinson added a comment.<br>
<br>
In <a href="https://reviews.llvm.org/D37364#860065" rel="noreferrer" target="_blank">https://reviews.llvm.org/D37364#860065</a>, @danielcdh wrote:<br>
<br>
> The change looks good to me. But the test is quite large and hard to understand. I don't think it's necessary to have the test generated from a source file, just forge a test with discriminator attached to line 0, and then checks if discriminators is omitted in the generated code. I guess it should suffice with < 20 LOC<br>
<br>
<br>
If the IR explicitly has a line 0 location with a nonzero discriminator, I think DwarfDebug should obediently emit that (or assert).<br>
The point of this patch is that when DwarfDebug *implicitly* emits a line 0 record, a preceding discriminator should not "pollute" that.<br></blockquote><div><br>Same idea, though - test might be easier to understand if it's a bit hand-crafted. (mostly writing all the debug metadata's the thing avoided by generating from C or C++ source - tweaking a few instructions (especially if it gets down to only a handful, like most LLVM IR (non-debug info & debug info alike) test cases))<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
<a href="https://reviews.llvm.org/D37364" rel="noreferrer" target="_blank">https://reviews.llvm.org/D37364</a><br>
<br>
<br>
<br>
</blockquote></div></div>