PATCH: private ivars

[Eric Christopher]

On Mon, Feb 18, 2013 at 2:52 PM, Adrian Prantl <aprantl at apple.com> wrote:

>
> On Feb 15, 2013, at 5:34 PM, jahanian <fjahanian at apple.com> wrote:
>
> > 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
> > ivars in class extensions. There is no guarantee that debug info. for
> primary and all class extensions are generated all the the same time.
> > This is the test case I am thinking:
>
> 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).
>

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?

+  // Do not cache the type if it may be incomplete
+  if (maybeIncompleteInterface(Ty))
+    return Res;

Maybe you could cache the type and add additional ivars as they're added to
the interface?

While you're going through some of these and other questions, couple of
nits on the patch:

metadata !{i32 786445

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.

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.

+  // Do not cache the type if it may be incomplete

Complete sentences (ok, just the '.').

+  // If the implementation is not yet set, we do not want to mark it
+  // as complete. An implementation may declare additional
+  // private ivars that we would miss otherwise.
+  if (ID->getImplementation() == 0) {
+    CompletedTypeCache.erase(QualTy.getAsOpaquePtr());
+    //incompleteInterfaces.push_back(std::make_pair(Ty, Unit));
+  }

No commented out code please.

-  CompletedTypeCache[QualType(Ty, 0).getAsOpaquePtr()] = RealDecl;
+  QualType QualTy = QualType(Ty, 0);
+  CompletedTypeCache[QualTy.getAsOpaquePtr()] = RealDecl;

Hmm?

+/// Caveat: If you invoke this method before the implementations was
+/// the list of ivars will be incomplete. If you call it again after

Not sure that sentence makes a lot of sense?

-eric
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130218/36462aab/attachment.html>