are there any remaining issues with this patch that you'd like me to fix? thanks, Adrian
On Feb 19, 2013, at 10:35 AM, Adrian Prantl <[email protected]> wrote: > > On Feb 18, 2013, at 10:47 PM, Eric Christopher <[email protected]> wrote: > >> 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? > > My understanding is that MD nodes are uniqued, is that correct? Based on that > assumption we are creating the DIType for the ObjCInterfaceDecl twice, but it > will point to the same MDNode. The second time around we are, however, adding > a new DW_TAG_member for each additional ivar. > >> >> + // 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? > > Possibly, but keep in mind that the ivar cache is being rebuilt with each > extension anyway, this seems to be the least invasive way to do this. >> >> 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. > fixed. >> 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 '.') > > fixed. > >> + // 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)); >> + } > > fixed. > >> - CompletedTypeCache[QualType(Ty, 0).getAsOpaquePtr()] = RealDecl; >> + QualType QualTy = QualType(Ty, 0); >> + CompletedTypeCache[QualTy.getAsOpaquePtr()] = RealDecl; >> >> Hmm? > > I'm reusing QualTy later on. > >> >> +/// Caveat: If you invoke this method before the implementations was >> +/// the list of ivars will be incomplete. If you call it again after >> > > fixed. > > > thanks, > Adrian > > > <private-ivars.patch>_______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
