<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Feb 11, 2014 at 2:47 PM, Adrian Prantl <span dir="ltr"><<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div><br>
On Feb 11, 2014, at 14:10, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>
<br>
><br>
><br>
><br>
> On Tue, Feb 11, 2014 at 1:47 PM, Adrian Prantl <<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>> wrote:<br>
><br>
> On Feb 10, 2014, at 23:15, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>
><br>
> > So I'm trying to do some stuff that's affecting .loc directives and came across a couple of test cases in Clang that, as was the fashion at the time, test LLVM.<br>
> ><br>
> > One of them is CodeGenObjC/debug-property-synth.m - which tests a particular .loc directive. The corresponding change it's meant to test is r131701, which seems to actually contain two independent changes. <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjC.cpp?r1=131373&r2=131701&pathrev=131701&diff_format=h" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjC.cpp?r1=131373&r2=131701&pathrev=131701&diff_format=h</a><br>


><br>
> Wonderfully documented testcase :-p<br>
><br>
> From looking at the radar, this test is supposed to test the location of synthesized getters and setters. We should not remove the testcase, but we should definitely make it more specific.<br>
><br>
> It should be testing that the synthesized setter/getter function for<br>
>      1  // RUN: %clang_cc1 -triple %itanium_abi_triple -masm-verbose -S -g %s -o - | FileCheck %s<br>
>      2  // Radar 9468526<br>
>      3  @interface I {<br>
>      4    int _p1;<br>
>      5  }<br>
>      6  @property int p1;<br>
>      7  @end<br>
>      8<br>
>      9  @implementation I<br>
>     10  @synthesize p1 = _p1;<br>
>     11  @end<br>
>     12<br>
>     13  int main() {<br>
>     14    I *myi;<br>
>     15    myi.p1 = 2;<br>
>     16    return 0;<br>
>     17  }<br>
>     18<br>
>     19  // FIXME: Make this test ir files.<br>
>     20  // CHECK:       .loc    2 6 0<br>
><br>
> p1 are associated with line number 6.<br>
><br>
> ><br>
> > 1) is the change at line 142 (and most of the changes in this patch - to pass the parameter through) to pass StartLoc instead of OMD->getLocStart()<br>
> ><br>
> > 2) is the change at 492 to use PID instead of PD<br>
> ><br>
><br>
> What are the respective values for StartLoc versus OMD->getLocStart() for this example?<br>
><br>
> That's the problem - the code has since change substantially. Mostly in r139468: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjC.cpp?r1=139466&r2=139468&diff_format=h" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjC.cpp?r1=139466&r2=139468&diff_format=h</a><br>


><br>
> When I attempted to possibly reproduce the failure (by finding the one place that r139468 kept a getLocStart call in GenerateObjCSetter, I think I was only able to get debuginfo-properties.m to fail)<br>
><br>
> So my best guess is that this test became useless at some point (possibly at 139468) due to refactoring and that the issue is currently covered by debuginfo-properties.m.<br>
<br>
</div></div>AFAICT debuginfo-properties.m checks for the DW_AT_decl_line in the debug info, whereas this tests for a line table entry. I forgot to mention that the radar was about being able to set a breakpoint on the property.</blockquote>

<div><br></div><div>OK - though I wasn't able to get the test case to fail. I suspect because something else ended up with the same line information, perhaps? Or it ended up with the correct line info through some other means? (I didn't diff the full output to see if it changed)<br>
<br>The commit the test case was addressing didn't seem like it was doing anything line-table specific... I assume it was just testing the line table because it was a convenient way to test the line associated with this construct (I assume this was originally a grep test long before llvm-dwarfdump for example).</div>

</div></div></div>