<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Feb 18, 2013 at 2:52 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:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="im"><br>
On Feb 15, 2013, at 5:34 PM, jahanian <<a href="mailto:fjahanian@apple.com">fjahanian@apple.com</a>> wrote:<br>
<br>
> Thanks for the explanation Adrian. Have you thought about the scenario that Debug info. is generated for primary class and then this routine is called for<br>
> ivars in class extensions. There is no guarantee that debug info. for primary and all class extensions are generated all the the same time.<br>
> This is the test case I am thinking:<br>
<br>
</div>Good catch. Here is an update that also includes your example (as debug-info-ivars-private-2.m). I now also disable caching of DebugInfo Types for pointers to Interfaces, which takes care of indirect uses of the interface as well (cf. debug-info-ivars-private-3.m).<br>
</blockquote><div><br></div><div style>If you've disabled caching on the types is there a case when you will have duplicate debug info? What kind of debug info output are you expecting out of the backend for what you're giving it?</div>
<div style><br></div><div style><div>+  // Do not cache the type if it may be incomplete</div><div>+  if (maybeIncompleteInterface(Ty))</div><div>+    return Res;</div><div><br></div><div>Maybe you could cache the type and add additional ivars as they're added to the interface?</div>
</div><div style><br></div><div style>While you're going through some of these and other questions, couple of nits on the patch:</div><div style><br></div><div style>metadata !{i32 786445</div><div style><br></div><div style>
the first field is really almost never needed to be checked and will make it more difficult to change later if we decide to change the format.</div><div style><br></div><div style>The radar number isn't useful in the testcase, I'd prefer a much longer description of what you're fixing and what the correct behavior should be for the test.</div>
<div style><br></div><div>+  // Do not cache the type if it may be incomplete</div><div style><br></div><div style>Complete sentences (ok, just the '.').</div><div style><br></div><div>+  // If the implementation is not yet set, we do not want to mark it</div>
<div>+  // as complete. An implementation may declare additional</div><div>+  // private ivars that we would miss otherwise.</div><div>+  if (ID->getImplementation() == 0) {</div><div>+    CompletedTypeCache.erase(QualTy.getAsOpaquePtr());</div>
<div>+    //incompleteInterfaces.push_back(std::make_pair(Ty, Unit));</div><div>+  }</div><div style><br></div><div style>No commented out code please.</div><div style><br></div><div>-  CompletedTypeCache[QualType(Ty, 0).getAsOpaquePtr()] = RealDecl;</div>
<div>+  QualType QualTy = QualType(Ty, 0);</div><div>+  CompletedTypeCache[QualTy.getAsOpaquePtr()] = RealDecl;</div><div style><br></div><div style>Hmm?</div><div style><br></div><div>+/// Caveat: If you invoke this method before the implementations was</div>
<div>+/// the list of ivars will be incomplete. If you call it again after</div><div style><br></div><div style>Not sure that sentence makes a lot of sense?</div><div style><br></div><div style>-eric </div></div></div></div>