<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div>This comment for ObjCImplementationDecl is totally bogus now, BTW.</div><div><br></div><div><div style="margin: 0px; font-size: 11px; font-family: Menlo; color: rgb(0, 132, 0);">/// Typically, instance variables are specified in the class interface,</div><div style="margin: 0px; font-size: 11px; font-family: Menlo; color: rgb(0, 132, 0);">/// *not* in the implementation. Nevertheless (for legacy reasons), we</div><div style="margin: 0px; font-size: 11px; font-family: Menlo; color: rgb(0, 132, 0);">/// allow instance variables to be specified in the implementation. When</div><div style="margin: 0px; font-size: 11px; font-family: Menlo; color: rgb(0, 132, 0);">/// specified, they need to be *identical* to the interface.</div><div style="margin: 0px; font-size: 11px; font-family: Menlo; color: rgb(0, 132, 0);">///</div></div><div><br></div><br><div><div>On May 5, 2014, at 16:38, Jordan Rose <<a href="mailto:jordan_rose@apple.com">jordan_rose@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">@interface is a definition of an ObjC class, but it doesn't contain all the ivars. @implementation can also contain ivars, but it's not in the redeclaration chain for the @interface/@class. Is this going to mess things up at all?<br><br>Jordan<br><br><br>On May 5, 2014, at 16:23, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br><br><blockquote type="cite">Author: dblaikie<br>Date: Mon May 5 18:23:53 2014<br>New Revision: 208015<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=208015&view=rev">http://llvm.org/viewvc/llvm-project?rev=208015&view=rev</a><br>Log:<br>Build debug info for ObjC interface types at the end of the translation unit to ensure all ivars are included.<br><br>This takes a different approach than the<br>completedType/requiresCompleteType work which relies on AST callbacks to<br>upgrade the type declaration to a definition. Instead, just defer<br>constructing the definition to the end of the translation unit.<br><br>This works because the definition is never needed by other debug info<br>(so far as I know), whereas the definition of a struct may be needed by<br>other debug info before the end of the translation unit (such as<br>emitting the definition of a member function which must refer to that<br>member function's declaration).<br><br>If we had a callback for whenever an IVar was added to an ObjC interface<br>we could use that, and remove the need for the ObjCInterfaceCache, which<br>might be nice. (also would need a callback for when it was more than<br>just a declaration so we could get properties, etc).<br><br>A side benefit is that we also don't need the CompletedTypeCache<br>anymore. Just rely on the declaration-ness of a type to decide whether<br>its definition is yet to be emitted.<br><br>There's still the PR19562 memory leak, but this should hopefully make<br>that a bit easier to approach.<br><br>Modified:<br> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp<br> cfe/trunk/lib/CodeGen/CGDebugInfo.h<br> cfe/trunk/test/CodeGenObjC/debug-info-ivars-indirect.m<br><br>Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=208015&r1=208014&r2=208015&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=208015&r1=208014&r2=208015&view=diff</a><br>==============================================================================<br>--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)<br>+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon May 5 18:23:53 2014<br>@@ -1453,11 +1453,13 @@ void CGDebugInfo::completeClassData(cons<br> return;<br> QualType Ty = CGM.getContext().getRecordType(RD);<br> void* TyPtr = Ty.getAsOpaquePtr();<br>- if (CompletedTypeCache.count(TyPtr))<br>+ auto I = TypeCache.find(TyPtr);<br>+ if (I != TypeCache.end() &&<br>+ !llvm::DIType(cast<llvm::MDNode>(static_cast<llvm::Value *>(I->second)))<br>+ .isForwardDecl())<br> return;<br> llvm::DIType Res = CreateTypeDefinition(Ty->castAs<RecordType>());<br> assert(!Res.isForwardDecl());<br>- CompletedTypeCache[TyPtr] = Res;<br> TypeCache[TyPtr] = Res;<br>}<br><br>@@ -1545,9 +1547,6 @@ llvm::DIType CGDebugInfo::CreateTypeDefi<br> LexicalBlockStack.push_back(&*FwdDecl);<br> RegionMap[Ty->getDecl()] = llvm::WeakVH(FwdDecl);<br><br>- // Add this to the completed-type cache while we're completing it recursively.<br>- CompletedTypeCache[QualType(Ty, 0).getAsOpaquePtr()] = FwdDecl;<br>-<br> // Convert all the elements.<br> SmallVector<llvm::Value *, 16> EltTys;<br> // what about nested types?<br>@@ -1624,15 +1623,24 @@ llvm::DIType CGDebugInfo::CreateType(con<br> // If this is just a forward declaration return a special forward-declaration<br> // debug type since we won't be able to lay out the entire type.<br> ObjCInterfaceDecl *Def = ID->getDefinition();<br>- if (!Def) {<br>+ if (!Def || !Def->getImplementation()) {<br> llvm::DIType FwdDecl =<br> DBuilder.createForwardDecl(llvm::dwarf::DW_TAG_structure_type,<br> ID->getName(), TheCU, DefUnit, Line,<br> RuntimeLang);<br>+ ObjCInterfaceCache.push_back(ObjCInterfaceCacheEntry(Ty, FwdDecl, Unit));<br> return FwdDecl;<br> }<br><br>- ID = Def;<br>+<br>+ return CreateTypeDefinition(Ty, Unit);<br>+}<br>+<br>+llvm::DIType CGDebugInfo::CreateTypeDefinition(const ObjCInterfaceType *Ty, llvm::DIFile Unit) {<br>+ ObjCInterfaceDecl *ID = Ty->getDecl();<br>+ llvm::DIFile DefUnit = getOrCreateFile(ID->getLocation());<br>+ unsigned Line = getLineNumber(ID->getLocation());<br>+ unsigned RuntimeLang = TheCU.getLanguage();<br><br> // Bit size, align and offset of the type.<br> uint64_t Size = CGM.getContext().getTypeSize(Ty);<br>@@ -1647,10 +1655,8 @@ llvm::DIType CGDebugInfo::CreateType(con<br> Line, Size, Align, Flags,<br> llvm::DIType(), llvm::DIArray(), RuntimeLang);<br><br>- // Otherwise, insert it into the CompletedTypeCache so that recursive uses<br>- // will find it and we're emitting the complete type.<br>- QualType QualTy = QualType(Ty, 0);<br>- CompletedTypeCache[QualTy.getAsOpaquePtr()] = RealDecl;<br>+ QualType QTy(Ty, 0);<br>+ TypeCache[QTy.getAsOpaquePtr()] = RealDecl;<br><br> // Push the struct on region stack.<br> LexicalBlockStack.push_back(static_cast<llvm::MDNode*>(RealDecl));<br>@@ -1774,12 +1780,6 @@ llvm::DIType CGDebugInfo::CreateType(con<br> llvm::DIArray Elements = DBuilder.getOrCreateArray(EltTys);<br> RealDecl.setTypeArray(Elements);<br><br>- // If the implementation is not yet set, we do not want to mark it<br>- // as complete. An implementation may declare additional<br>- // private ivars that we would miss otherwise.<br>- if (ID->getImplementation() == 0)<br>- CompletedTypeCache.erase(QualTy.getAsOpaquePtr());<br>-<br> LexicalBlockStack.pop_back();<br> return RealDecl;<br>}<br>@@ -2002,14 +2002,6 @@ llvm::DIType CGDebugInfo::getTypeOrNull(<br> // Unwrap the type as needed for debug information.<br> Ty = UnwrapTypeForDebugInfo(Ty, CGM.getContext());<br><br>- // Check for existing entry.<br>- if (Ty->getTypeClass() == Type::ObjCInterface) {<br>- llvm::Value *V = getCachedInterfaceTypeOrNull(Ty);<br>- if (V)<br>- return llvm::DIType(cast<llvm::MDNode>(V));<br>- else return llvm::DIType();<br>- }<br>-<br> llvm::DenseMap<void *, llvm::WeakVH>::iterator it =<br> TypeCache.find(Ty.getAsOpaquePtr());<br> if (it != TypeCache.end()) {<br>@@ -2021,27 +2013,6 @@ llvm::DIType CGDebugInfo::getTypeOrNull(<br> return llvm::DIType();<br>}<br><br>-/// getCompletedTypeOrNull - Get the type from the cache or return null if it<br>-/// doesn't exist.<br>-llvm::DIType CGDebugInfo::getCompletedTypeOrNull(QualType Ty) {<br>-<br>- // Unwrap the type as needed for debug information.<br>- Ty = UnwrapTypeForDebugInfo(Ty, CGM.getContext());<br>-<br>- // Check for existing entry.<br>- llvm::Value *V = 0;<br>- llvm::DenseMap<void *, llvm::WeakVH>::iterator it =<br>- CompletedTypeCache.find(Ty.getAsOpaquePtr());<br>- if (it != CompletedTypeCache.end())<br>- V = it->second;<br>- else {<br>- V = getCachedInterfaceTypeOrNull(Ty);<br>- }<br>-<br>- // Verify that any cached debug info still exists.<br>- return llvm::DIType(cast_or_null<llvm::MDNode>(V));<br>-}<br>-<br>void CGDebugInfo::completeTemplateDefinition(<br> const ClassTemplateSpecializationDecl &SD) {<br> if (DebugKind <= CodeGenOptions::DebugLineTablesOnly)<br>@@ -2053,22 +2024,6 @@ void CGDebugInfo::completeTemplateDefini<br> RetainedTypes.push_back(CGM.getContext().getRecordType(&SD).getAsOpaquePtr());<br>}<br><br>-/// getCachedInterfaceTypeOrNull - Get the type from the interface<br>-/// cache, unless it needs to regenerated. Otherwise return null.<br>-llvm::Value *CGDebugInfo::getCachedInterfaceTypeOrNull(QualType Ty) {<br>- // Is there a cached interface that hasn't changed?<br>- llvm::DenseMap<void *, std::pair<llvm::WeakVH, unsigned > ><br>- ::iterator it1 = ObjCInterfaceCache.find(Ty.getAsOpaquePtr());<br>-<br>- if (it1 != ObjCInterfaceCache.end())<br>- if (ObjCInterfaceDecl* Decl = getObjCInterfaceDecl(Ty))<br>- if (Checksum(Decl) == it1->second.second)<br>- // Return cached forward declaration.<br>- return it1->second.first;<br>-<br>- return 0;<br>-}<br>-<br>/// getOrCreateType - Get the type from the cache or create a new<br>/// one if necessary.<br>llvm::DIType CGDebugInfo::getOrCreateType(QualType Ty, llvm::DIFile Unit) {<br>@@ -2078,7 +2033,7 @@ llvm::DIType CGDebugInfo::getOrCreateTyp<br> // Unwrap the type as needed for debug information.<br> Ty = UnwrapTypeForDebugInfo(Ty, CGM.getContext());<br><br>- if (llvm::DIType T = getCompletedTypeOrNull(Ty))<br>+ if (llvm::DIType T = getTypeOrNull(Ty))<br> return T;<br><br> // Otherwise create the type.<br>@@ -2088,37 +2043,6 @@ llvm::DIType CGDebugInfo::getOrCreateTyp<br> // And update the type cache.<br> TypeCache[TyPtr] = Res;<br><br>- // FIXME: this getTypeOrNull call seems silly when we just inserted the type<br>- // into the cache - but getTypeOrNull has a special case for cached interface<br>- // types. We should probably just pull that out as a special case for the<br>- // "else" block below & skip the otherwise needless lookup.<br>- llvm::DIType TC = getTypeOrNull(Ty);<br>- if (ObjCInterfaceDecl *Decl = getObjCInterfaceDecl(Ty)) {<br>- // Interface types may have elements added to them by a<br>- // subsequent implementation or extension, so we keep them in<br>- // the ObjCInterfaceCache together with a checksum. Instead of<br>- // the (possibly) incomplete interface type, we return a forward<br>- // declaration that gets RAUW'd in CGDebugInfo::finalize().<br>- std::pair<llvm::WeakVH, unsigned> &V = ObjCInterfaceCache[TyPtr];<br>- if (V.first)<br>- return llvm::DIType(cast<llvm::MDNode>(V.first));<br>- TC = DBuilder.createForwardDecl(llvm::dwarf::DW_TAG_structure_type,<br>- Decl->getName(), TheCU, Unit,<br>- getLineNumber(Decl->getLocation()),<br>- TheCU.getLanguage());<br>- // Store the forward declaration in the cache.<br>- V.first = TC;<br>- V.second = Checksum(Decl);<br>-<br>- // Register the type for replacement in finalize().<br>- ReplaceMap.push_back(std::make_pair(TyPtr, static_cast<llvm::Value*>(TC)));<br>-<br>- return TC;<br>- }<br>-<br>- if (!Res.isForwardDecl())<br>- CompletedTypeCache[TyPtr] = Res;<br>-<br> return Res;<br>}<br><br>@@ -2282,13 +2206,6 @@ llvm::DICompositeType CGDebugInfo::Creat<br><br> // If we ended up creating the type during the context chain construction,<br> // just return that.<br>- // FIXME: this could be dealt with better if the type was recorded as<br>- // completed before we started this (see the CompletedTypeCache usage in<br>- // CGDebugInfo::CreateTypeDefinition(const RecordType*) - that would need to<br>- // be pushed to before context creation, but after it was known to be<br>- // destined for completion (might still have an issue if this caller only<br>- // required a declaration but the context construction ended up creating a<br>- // definition)<br> llvm::DICompositeType T(getTypeOrNull(CGM.getContext().getRecordType(RD)));<br> if (T && (!T.isForwardDecl() || !RD->getDefinition()))<br> return T;<br>@@ -3393,6 +3310,11 @@ void CGDebugInfo::finalize() {<br> Ty.replaceAllUsesWith(RepTy);<br> }<br><br>+ for (auto E : ObjCInterfaceCache)<br>+ E.Decl.replaceAllUsesWith(E.Type->getDecl()->getDefinition()<br>+ ? CreateTypeDefinition(E.Type, E.Unit)<br>+ : E.Decl);<br>+<br> // We keep our own list of retained types, because we need to look<br> // up the final type in the type cache.<br> for (std::vector<void *>::const_iterator RI = RetainedTypes.begin(),<br><br>Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.h<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=208015&r1=208014&r2=208015&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=208015&r1=208014&r2=208015&view=diff</a><br>==============================================================================<br>--- cfe/trunk/lib/CodeGen/CGDebugInfo.h (original)<br>+++ cfe/trunk/lib/CodeGen/CGDebugInfo.h Mon May 5 18:23:53 2014<br>@@ -67,16 +67,22 @@ class CGDebugInfo {<br> /// TypeCache - Cache of previously constructed Types.<br> llvm::DenseMap<void *, llvm::WeakVH> TypeCache;<br><br>+ struct ObjCInterfaceCacheEntry {<br>+ const ObjCInterfaceType *Type;<br>+ llvm::DIType Decl;<br>+ llvm::DIFile Unit;<br>+ ObjCInterfaceCacheEntry(const ObjCInterfaceType *Type, llvm::DIType Decl,<br>+ llvm::DIFile Unit)<br>+ : Type(Type), Decl(Decl), Unit(Unit) {}<br>+ };<br>+<br> /// ObjCInterfaceCache - Cache of previously constructed interfaces<br>- /// which may change. Storing a pair of DIType and checksum.<br>- llvm::DenseMap<void *, std::pair<llvm::WeakVH, unsigned> > ObjCInterfaceCache;<br>+ /// which may change.<br>+ llvm::SmallVector<ObjCInterfaceCacheEntry, 32> ObjCInterfaceCache;<br><br> /// RetainedTypes - list of interfaces we want to keep even if orphaned.<br> std::vector<void *> RetainedTypes;<br><br>- /// CompleteTypeCache - Cache of previously constructed complete RecordTypes.<br>- llvm::DenseMap<void *, llvm::WeakVH> CompletedTypeCache;<br>-<br> /// ReplaceMap - Cache of forward declared types to RAUW at the end of<br> /// compilation.<br> std::vector<std::pair<void *, llvm::WeakVH> >ReplaceMap;<br>@@ -120,6 +126,7 @@ class CGDebugInfo {<br> llvm::DICompositeType CreateLimitedType(const RecordType *Ty);<br> void CollectContainingType(const CXXRecordDecl *RD, llvm::DICompositeType CT);<br> llvm::DIType CreateType(const ObjCInterfaceType *Ty, llvm::DIFile F);<br>+ llvm::DIType CreateTypeDefinition(const ObjCInterfaceType *Ty, llvm::DIFile F);<br> llvm::DIType CreateType(const ObjCObjectType *Ty, llvm::DIFile F);<br> llvm::DIType CreateType(const VectorType *Ty, llvm::DIFile F);<br> llvm::DIType CreateType(const ArrayType *Ty, llvm::DIFile F);<br>@@ -130,7 +137,6 @@ class CGDebugInfo {<br> llvm::DIType CreateEnumType(const EnumType *Ty);<br> llvm::DIType CreateSelfType(const QualType &QualTy, llvm::DIType Ty);<br> llvm::DIType getTypeOrNull(const QualType);<br>- llvm::DIType getCompletedTypeOrNull(const QualType);<br> llvm::DICompositeType getOrCreateMethodType(const CXXMethodDecl *Method,<br> llvm::DIFile F);<br> llvm::DICompositeType getOrCreateInstanceMethodType(<br><br>Modified: cfe/trunk/test/CodeGenObjC/debug-info-ivars-indirect.m<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/debug-info-ivars-indirect.m?rev=208015&r1=208014&r2=208015&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/debug-info-ivars-indirect.m?rev=208015&r1=208014&r2=208015&view=diff</a><br>==============================================================================<br>--- cfe/trunk/test/CodeGenObjC/debug-info-ivars-indirect.m (original)<br>+++ cfe/trunk/test/CodeGenObjC/debug-info-ivars-indirect.m Mon May 5 18:23:53 2014<br>@@ -3,6 +3,14 @@<br>// Make sure we generate debug symbols for an indirectly referenced<br>// extension to an interface.<br><br>+// This happens to be the order the members are emitted in... I'm assuming it's<br>+// not meaningful/important, so if something causes the order to change, feel<br>+// free to update the test to reflect the new order.<br>+// CHECK: ; [ DW_TAG_member ] [a]<br>+// CHECK: ; [ DW_TAG_member ] [d]<br>+// CHECK: ; [ DW_TAG_member ] [c]<br>+// CHECK: ; [ DW_TAG_member ] [b]<br>+<br>@interface I<br>{<br> @public int a;<br>@@ -29,7 +37,6 @@ void gorf (struct S* s) {<br> int _b = s->i->b;<br>}<br><br>-// CHECK: ; [ DW_TAG_member ] [b]<br><br>I *source();<br><br>@@ -39,8 +46,14 @@ I *source();<br>}<br>@end<br><br>-// CHECK: ; [ DW_TAG_member ] [c]<br>-<br>void use() {<br> int _c = source()->c;<br>}<br>+<br>+@interface I()<br>+{<br>+ @public int d;<br>+}<br>+@end<br>+<br>+I *x();<br><br><br>_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits<br></blockquote><br></blockquote></div><br></body></html>