<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Feb 11, 2015, at 11:27 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Wed, Feb 11, 2015 at 11:14 AM, Adrian Prantl <span dir="ltr" class=""><<a href="mailto:aprantl@apple.com" target="_blank" class="">aprantl@apple.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><br class=""><div class=""><span class=""><blockquote type="cite" class=""><div class="">On Feb 11, 2015, at 11:06 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank" class="">dblaikie@gmail.com</a>> wrote:</div><br class=""><div class=""><div dir="ltr" class=""><br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Wed, Feb 11, 2015 at 10:28 AM, Adrian Prantl <span dir="ltr" class=""><<a href="mailto:aprantl@apple.com" target="_blank" class="">aprantl@apple.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><br class=""><div class=""><span class=""><blockquote type="cite" class=""><div class="">On Feb 11, 2015, at 10:21 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank" class="">dblaikie@gmail.com</a>> wrote:</div><br class=""><div class=""><div dir="ltr" class=""><br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Wed, Feb 11, 2015 at 9:45 AM, Adrian Prantl <span dir="ltr" class=""><<a href="mailto:aprantl@apple.com" target="_blank" class="">aprantl@apple.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: adrian<br class="">
Date: Wed Feb 11 11:45:15 2015<br class="">
New Revision: 228855<br class="">
<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=228855&view=rev" target="_blank" class="">http://llvm.org/viewvc/llvm-project?rev=228855&view=rev</a><br class="">
Log:<br class="">
Fix PR19351. While building up a composite type it is important to use<br class="">
a non-uniqueable temporary node that is only turned into a permanent<br class="">
unique or distinct node after it is finished.<br class="">
Otherwise an intermediate node may get accidentally uniqued with another<br class="">
node as illustrated by the testcase.<br class=""></blockquote><div class=""><br class="">Awesome - thanks!<br class=""> </div></div></div></div></div></blockquote><div class=""><br class=""></div></span><div class="">We should probably rename getOrCreateLimitedType() now, as it has nothing to do with limited debug info any more. Any suggestions?</div></div></div></blockquote></div></div></div></div></blockquote><div class=""><br class=""></div><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""><br class="">I didn't catch the impact your change had on that function. What does it have to do with now?<br class=""></div></div></div></div></div></blockquote><div class=""><br class=""></div></span><div class="">“Now” was not actually meant as “after this commit” but rather as “today’s CFE”. This is just something that I happened noticed while working on this, sorry for being misleading there :-)</div></div></div></blockquote><div class=""><br class="">'sok. I hadn't noticed/thought about it - what's it do now compared to what it did?<br class=""></div></div></div></div></div></blockquote><div><br class=""></div>I think this used to be the entry point for creating types for limited debug info, but it is really only used to create the temporary shells of what may become recursive composite types.</div><div><br class=""></div><div>-- adrian<br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><div class=""><span class="HOEnZb"><font color="#888888" class=""><div class=""><br class=""></div><div class="">-- adrian</div></font></span><div class=""><div class="h5"><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><div class=""><span class=""><font color="#888888" class=""><div class=""><br class=""></div><div class="">-- adrian</div></font></span><div class=""><div class=""><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br class="">
Paired commit with LLVM.<br class="">
<br class="">
Added:<br class="">
    cfe/trunk/test/CodeGen/debug-info-same-line.c<br class="">
Modified:<br class="">
    cfe/trunk/lib/CodeGen/CGDebugInfo.cpp<br class="">
<br class="">
Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=228855&r1=228854&r2=228855&view=diff" target="_blank" class="">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=228855&r1=228854&r2=228855&view=diff</a><br class="">
==============================================================================<br class="">
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)<br class="">
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Wed Feb 11 11:45:15 2015<br class="">
@@ -621,6 +621,21 @@ static SmallString<256> getUniqueTagType<br class="">
   return FullName;<br class="">
 }<br class="">
<br class="">
+static llvm::dwarf::Tag getTagForRecord(const RecordDecl *RD) {<br class="">
+   llvm::dwarf::Tag Tag;<br class="">
+  if (RD->isStruct() || RD->isInterface())<br class="">
+    Tag = llvm::dwarf::DW_TAG_structure_type;<br class="">
+  else if (RD->isUnion())<br class="">
+    Tag = llvm::dwarf::DW_TAG_union_type;<br class="">
+  else {<br class="">
+    // FIXME: This could be a struct type giving a default visibility different<br class="">
+    // than C++ class type, but needs llvm metadata changes first.<br class="">
+    assert(RD->isClass());<br class="">
+    Tag = llvm::dwarf::DW_TAG_class_type;<br class="">
+  }<br class="">
+  return Tag;<br class="">
+}<br class="">
+<br class="">
 // Creates a forward declaration for a RecordDecl in the given context.<br class="">
 llvm::DICompositeType<br class="">
 CGDebugInfo::getOrCreateRecordFwdDecl(const RecordType *Ty,<br class="">
@@ -632,20 +647,12 @@ CGDebugInfo::getOrCreateRecordFwdDecl(co<br class="">
   unsigned Line = getLineNumber(RD->getLocation());<br class="">
   StringRef RDName = getClassName(RD);<br class="">
<br class="">
-  llvm::dwarf::Tag Tag;<br class="">
-  if (RD->isStruct() || RD->isInterface())<br class="">
-    Tag = llvm::dwarf::DW_TAG_structure_type;<br class="">
-  else if (RD->isUnion())<br class="">
-    Tag = llvm::dwarf::DW_TAG_union_type;<br class="">
-  else {<br class="">
-    assert(RD->isClass());<br class="">
-    Tag = llvm::dwarf::DW_TAG_class_type;<br class="">
-  }<br class="">
<br class="">
   // Create the type.<br class="">
   SmallString<256> FullName = getUniqueTagTypeName(Ty, CGM, TheCU);<br class="">
-  llvm::DICompositeType RetTy = DBuilder.createReplaceableForwardDecl(<br class="">
-      Tag, RDName, Ctx, DefUnit, Line, 0, 0, 0, FullName);<br class="">
+  llvm::DICompositeType RetTy = DBuilder.createReplaceableCompositeType(<br class="">
+      getTagForRecord(RD), RDName, Ctx, DefUnit, Line, 0, 0, 0,<br class="">
+      llvm::DIDescriptor::FlagFwdDecl, FullName);<br class="">
   ReplaceMap.emplace_back(<br class="">
       std::piecewise_construct, std::make_tuple(Ty),<br class="">
       std::make_tuple(static_cast<llvm::Metadata *>(RetTy)));<br class="">
@@ -1567,7 +1574,8 @@ llvm::DIType CGDebugInfo::CreateTypeDefi<br class="">
   assert(FwdDecl.isCompositeType() &&<br class="">
          "The debug type of a RecordType should be a llvm::DICompositeType");<br class="">
<br class="">
-  if (FwdDecl.isForwardDecl())<br class="">
+  const RecordDecl *D = RD->getDefinition();<br class="">
+  if (!D || !D->isCompleteDefinition())<br class="">
     return FwdDecl;<br class="">
<br class="">
   if (const CXXRecordDecl *CXXDecl = dyn_cast<CXXRecordDecl>(RD))<br class="">
@@ -1602,6 +1610,10 @@ llvm::DIType CGDebugInfo::CreateTypeDefi<br class="">
   llvm::DIArray Elements = DBuilder.getOrCreateArray(EltTys);<br class="">
   DBuilder.replaceArrays(FwdDecl, Elements);<br class="">
<br class="">
+  if (FwdDecl->isTemporary())<br class="">
+    FwdDecl = llvm::DICompositeType(llvm::MDNode::replaceWithPermanent(<br class="">
+      llvm::TempMDNode(FwdDecl.get())));<br class="">
+<br class="">
   RegionMap[Ty->getDecl()].reset(FwdDecl);<br class="">
   return FwdDecl;<br class="">
 }<br class="">
@@ -1653,7 +1665,7 @@ llvm::DIType CGDebugInfo::CreateType(con<br class="">
   // debug type since we won't be able to lay out the entire type.<br class="">
   ObjCInterfaceDecl *Def = ID->getDefinition();<br class="">
   if (!Def || !Def->getImplementation()) {<br class="">
-    llvm::DIType FwdDecl = DBuilder.createReplaceableForwardDecl(<br class="">
+    llvm::DIType FwdDecl = DBuilder.createReplaceableCompositeType(<br class="">
         llvm::dwarf::DW_TAG_structure_type, ID->getName(), TheCU, DefUnit, Line,<br class="">
         RuntimeLang);<br class="">
     ObjCInterfaceCache.push_back(ObjCInterfaceCacheEntry(Ty, FwdDecl, Unit));<br class="">
@@ -1933,9 +1945,9 @@ llvm::DIType CGDebugInfo::CreateEnumType<br class="">
     llvm::DIFile DefUnit = getOrCreateFile(ED->getLocation());<br class="">
     unsigned Line = getLineNumber(ED->getLocation());<br class="">
     StringRef EDName = ED->getName();<br class="">
-    llvm::DIType RetTy = DBuilder.createReplaceableForwardDecl(<br class="">
+    llvm::DIType RetTy = DBuilder.createReplaceableCompositeType(<br class="">
         llvm::dwarf::DW_TAG_enumeration_type, EDName, EDContext, DefUnit, Line,<br class="">
-        0, Size, Align, FullName);<br class="">
+        0, Size, Align, llvm::DIDescriptor::FlagFwdDecl, FullName);<br class="">
     ReplaceMap.emplace_back(<br class="">
         std::piecewise_construct, std::make_tuple(Ty),<br class="">
         std::make_tuple(static_cast<llvm::Metadata *>(RetTy)));<br class="">
@@ -2249,19 +2261,8 @@ llvm::DICompositeType CGDebugInfo::Creat<br class="">
<br class="">
   SmallString<256> FullName = getUniqueTagTypeName(Ty, CGM, TheCU);<br class="">
<br class="">
-  if (RD->isUnion())<br class="">
-    RealDecl = DBuilder.createUnionType(RDContext, RDName, DefUnit, Line, Size,<br class="">
-                                        Align, 0, llvm::DIArray(), 0, FullName);<br class="">
-  else if (RD->isClass()) {<br class="">
-    // FIXME: This could be a struct type giving a default visibility different<br class="">
-    // than C++ class type, but needs llvm metadata changes first.<br class="">
-    RealDecl = DBuilder.createClassType(<br class="">
-        RDContext, RDName, DefUnit, Line, Size, Align, 0, 0, llvm::DIType(),<br class="">
-        llvm::DIArray(), llvm::DIType(), llvm::DIArray(), FullName);<br class="">
-  } else<br class="">
-    RealDecl = DBuilder.createStructType(<br class="">
-        RDContext, RDName, DefUnit, Line, Size, Align, 0, llvm::DIType(),<br class="">
-        llvm::DIArray(), 0, llvm::DIType(), FullName);<br class="">
+  RealDecl = DBuilder.createReplaceableCompositeType(getTagForRecord(RD),<br class="">
+      RDName, RDContext, DefUnit, Line, 0, Size, Align, 0, FullName);<br class="">
<br class="">
   RegionMap[Ty->getDecl()].reset(RealDecl);<br class="">
   TypeCache[QualType(Ty, 0).getAsOpaquePtr()].reset(RealDecl);<br class="">
<br class="">
Added: cfe/trunk/test/CodeGen/debug-info-same-line.c<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/debug-info-same-line.c?rev=228855&view=auto" target="_blank" class="">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/debug-info-same-line.c?rev=228855&view=auto</a><br class="">
==============================================================================<br class="">
--- cfe/trunk/test/CodeGen/debug-info-same-line.c (added)<br class="">
+++ cfe/trunk/test/CodeGen/debug-info-same-line.c Wed Feb 11 11:45:15 2015<br class="">
@@ -0,0 +1,7 @@<br class="">
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm %s -g -o - | FileCheck %s<br class="">
+// Here two temporary nodes are identical (but should not get uniqued) while<br class="">
+// building the full debug type.<br class="">
+typedef struct { long x; } foo; typedef struct {  foo *x; } bar;<br class="">
+// CHECK: [ DW_TAG_structure_type ] [line 4, size 64,<br class="">
+// CHECK: [ DW_TAG_structure_type ] [line 4, size 64,<br class="">
+bar b;<br class="">
<br class="">
<br class="">
_______________________________________________<br class="">
cfe-commits mailing list<br class="">
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank" class="">cfe-commits@cs.uiuc.edu</a><br class="">
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank" class="">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br class="">
</blockquote></div><br class=""></div></div>
</div></blockquote></div></div></div><br class=""></div></blockquote></div><br class=""></div></div>
</div></blockquote></div></div></div><br class=""></div></blockquote></div><br class=""></div></div>
</div></blockquote></div><br class=""></body></html>