<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jul 7, 2016 at 4:22 PM, Adrian McCarthy <span dir="ltr"><<a href="mailto:amccarth@google.com" target="_blank">amccarth@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">This patch was reverted because it breaks a test on the buildbots (that I've been unable to reproduce locally), so that's why you didn't seen anything. I'll try again to land the patch once I can fix and verify that test.<div><br></div><div>This patch is one part of the change. The other part is <a href="http://reviews.llvm.org/D21939" target="_blank">http://reviews.llvm.org/D21939</a>, which ensures this extra info is carried through for CodeView debug info.</div><div><br></div><div>I'm not familiar with DWARF type units (but am currently reading about them). I don't immediately see how this change would affect the calculation of the type signature.</div></div></blockquote><div><br>Actually we build the signature based on the mangled name of the type - not the DIEs that make up the type unit. So we want to ensure that the type unit for the same type comes out the same in any TU.<br><br>Since we weren't including unreferenced nested types, what we would do is not include nested types in the member list - that way when we build the type we don't include those nested types (we use the same technique for implicit special members and member function templates). Those elements end up in the other part of the DWARF that references the type unit - the same way declarations for defined members appear there (since DWARF type units don't provide a way to reference anything other than the type - so... anyway, long story).<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>The intent is to ensure that there is a type record for Bar in code like this:<br></div><div><br></div><div>struct Foo {</div><div> struct Bar {};</div><div>};</div><div><br></div><div>Without this change, Bar is omitted from the debug info metadata.</div></div></blockquote><div><br></div><div>Do you know how this will work with this case:<br><br>struct Foo {<br> struct Bar;</div><div>};<br><br>Where Bar is defined and used in one TU but not defined in the other? I don't think we have the ability to emit both a declaration and a definition of a type in the same translation unit, which would make it difficult to describe Foo the same in both translation units (since in the TU without Bar's definition Foo would contain a declaration, and in the TU with Bar's definition we'd need to emit the definition and perhaps have no way (currently) to also emit the declaration)<br><br>Also, from a debug info size perspective, it'd be unfortunate if we required all nested types to be fully defined/emitted (if we can support the decl/def situation above, perhaps we could make even inline nested type definitions appear as separate decl/def to keep the option to omit the nested type definition and keep debug info size small-ish (it'd still grow a bit by adding the declarations even when the type's unreferenced - so I'd sort of like to keep that for DWARF if reasonable))<br><br>Also: if consistency of type definition is important for CV: What happens with implicit special members and member function templates?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Thu, Jul 7, 2016 at 4:03 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br></span><div><div class="h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">This may cause problems for DWARF type unit consistency... <br><br>Under what conditions do nested types appear in the member list? (my simple test case on a fresh clang didn't seem to produce anything about the nested type: struct outer { struct inner { int i; }; int j; }; outer o; )<br></div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jul 6, 2016 at 7:46 AM, Adrian McCarthy via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: amccarth<br>
Date: Wed Jul 6 09:46:42 2016<br>
New Revision: 274628<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=274628&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=274628&view=rev</a><br>
Log:<br>
Include debug info for nested structs and classes<br>
<br>
This includes nested types in the member list, even if there are no members of that type. Note that structs and classes have themselves as an "implicit struct" as the first member, so we skip implicit ones.<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D21705" rel="noreferrer" target="_blank">http://reviews.llvm.org/D21705</a><br>
<br>
Modified:<br>
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp<br>
cfe/trunk/lib/CodeGen/CGDebugInfo.h<br>
cfe/trunk/test/CodeGenCXX/debug-info-dup-fwd-decl.cpp<br>
cfe/trunk/test/CodeGenCXX/debug-info-indirect-field-decl.cpp<br>
cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp<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=274628&r1=274627&r2=274628&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=274628&r1=274627&r2=274628&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)<br>
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Wed Jul 6 09:46:42 2016<br>
@@ -1095,6 +1095,13 @@ void CGDebugInfo::CollectRecordNormalFie<br>
elements.push_back(FieldType);<br>
}<br>
<br>
+void CGDebugInfo::CollectRecordNestedRecord(<br>
+ const RecordDecl *RD, SmallVectorImpl<llvm::Metadata *> &elements) {<br>
+ QualType Ty = CGM.getContext().getTypeDeclType(RD);<br>
+ llvm::DIType *nestedType = getOrCreateType(Ty, getOrCreateMainFile());<br>
+ elements.push_back(nestedType);<br>
+}<br>
+<br>
void CGDebugInfo::CollectRecordFields(<br>
const RecordDecl *record, llvm::DIFile *tunit,<br>
SmallVectorImpl<llvm::Metadata *> &elements,<br>
@@ -1131,6 +1138,9 @@ void CGDebugInfo::CollectRecordFields(<br>
<br>
// Bump field number for next field.<br>
++fieldNo;<br>
+ } else if (const auto *nestedRec = dyn_cast<CXXRecordDecl>(I)) {<br>
+ if (!nestedRec->isImplicit() && nestedRec->getDeclContext() == record)<br>
+ CollectRecordNestedRecord(nestedRec, elements);<br>
}<br>
}<br>
}<br>
@@ -3633,8 +3643,8 @@ void CGDebugInfo::EmitUsingDirective(con<br>
if (CGM.getCodeGenOpts().getDebugInfo() < codegenoptions::LimitedDebugInfo)<br>
return;<br>
const NamespaceDecl *NSDecl = UD.getNominatedNamespace();<br>
- if (!NSDecl->isAnonymousNamespace() ||<br>
- CGM.getCodeGenOpts().DebugExplicitImport) {<br>
+ if (!NSDecl->isAnonymousNamespace() ||<br>
+ CGM.getCodeGenOpts().DebugExplicitImport) {<br>
DBuilder.createImportedModule(<br>
getCurrentContextDescriptor(cast<Decl>(UD.getDeclContext())),<br>
getOrCreateNameSpace(NSDecl),<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=274628&r1=274627&r2=274628&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=274628&r1=274627&r2=274628&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/CodeGen/CGDebugInfo.h (original)<br>
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.h Wed Jul 6 09:46:42 2016<br>
@@ -254,6 +254,8 @@ class CGDebugInfo {<br>
llvm::DIFile *F,<br>
SmallVectorImpl<llvm::Metadata *> &E,<br>
llvm::DIType *RecordTy, const RecordDecl *RD);<br>
+ void CollectRecordNestedRecord(const RecordDecl *RD,<br>
+ SmallVectorImpl<llvm::Metadata *> &E);<br>
void CollectRecordFields(const RecordDecl *Decl, llvm::DIFile *F,<br>
SmallVectorImpl<llvm::Metadata *> &E,<br>
llvm::DICompositeType *RecordTy);<br>
<br>
Modified: cfe/trunk/test/CodeGenCXX/debug-info-dup-fwd-decl.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-dup-fwd-decl.cpp?rev=274628&r1=274627&r2=274628&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-dup-fwd-decl.cpp?rev=274628&r1=274627&r2=274628&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/CodeGenCXX/debug-info-dup-fwd-decl.cpp (original)<br>
+++ cfe/trunk/test/CodeGenCXX/debug-info-dup-fwd-decl.cpp Wed Jul 6 09:46:42 2016<br>
@@ -19,6 +19,6 @@ protected:<br>
<br>
Test t;<br>
<br>
-// CHECK: !DIDerivedType(tag: DW_TAG_pointer_type<br>
// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "data"<br>
+// CHECK: !DIDerivedType(tag: DW_TAG_pointer_type<br>
// CHECK-NOT: !DICompositeType(tag: DW_TAG_structure_type, name: "data"<br>
<br>
Modified: cfe/trunk/test/CodeGenCXX/debug-info-indirect-field-decl.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-indirect-field-decl.cpp?rev=274628&r1=274627&r2=274628&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-indirect-field-decl.cpp?rev=274628&r1=274627&r2=274628&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/CodeGenCXX/debug-info-indirect-field-decl.cpp (original)<br>
+++ cfe/trunk/test/CodeGenCXX/debug-info-indirect-field-decl.cpp Wed Jul 6 09:46:42 2016<br>
@@ -8,18 +8,18 @@ template <class T, int T::*ptr> class Fo<br>
struct Bar {<br>
int i1;<br>
// CHECK: ![[INT:[0-9]+]] = !DIBasicType(name: "int"<br>
- // CHECK: !DIDerivedType(tag: DW_TAG_member, scope:<br>
- // CHECK-SAME: line: [[@LINE+4]]<br>
- // CHECK-SAME: baseType: ![[UNION:[0-9]+]]<br>
- // CHECK-SAME: size: 32, align: 32, offset: 32<br>
- // CHECK: ![[UNION]] = distinct !DICompositeType(tag: DW_TAG_union_type,{{.*}} identifier: "_ZTSN3BarUt_E")<br>
+ // CHECK: ![[UNION:[0-9]+]] = distinct !DICompositeType(tag: DW_TAG_union_type,{{.*}} identifier: "_ZTSN3BarUt_E")<br>
union {<br>
// CHECK: !DIDerivedType(tag: DW_TAG_member, name: "i2",<br>
- // CHECK-SAME: line: [[@LINE+5]]<br>
+ // CHECK-SAME: line: [[@LINE+9]]<br>
// CHECK-SAME: baseType: ![[INT]]<br>
// CHECK-SAME: size: 32, align: 32<br>
// CHECK-NOT: offset:<br>
// CHECK-SAME: ){{$}}<br>
+ // CHECK: !DIDerivedType(tag: DW_TAG_member, scope:<br>
+ // CHECK-SAME: line: [[@LINE-8]]<br>
+ // CHECK-SAME: baseType: ![[UNION]]<br>
+ // CHECK-SAME: size: 32, align: 32, offset: 32<br>
int i2;<br>
};<br>
};<br>
<br>
Modified: cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp?rev=274628&r1=274627&r2=274628&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp?rev=274628&r1=274627&r2=274628&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp (original)<br>
+++ cfe/trunk/test/CodeGenCXX/debug-info-ms-abi.cpp Wed Jul 6 09:46:42 2016<br>
@@ -14,6 +14,9 @@ Foo::Nested n;<br>
// CHECK: ![[Foo:[^ ]*]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Foo",<br>
// CHECK-SAME: identifier: ".?AUFoo@@"<br>
<br>
+// CHECK: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Nested",<br>
+// CHECK-SAME: identifier: ".?AUNested@Foo@@"<br>
+<br>
// CHECK: !DISubprogram(name: "f",<br>
// CHECK-SAME: containingType: ![[Foo]], virtuality: DW_VIRTUALITY_virtual, virtualIndex: 0,<br>
// CHECK-SAME: flags: DIFlagPrototyped | DIFlagIntroducedVirtual,<br>
@@ -25,6 +28,3 @@ Foo::Nested n;<br>
// CHECK: !DISubprogram(name: "h",<br>
// CHECK-SAME: containingType: ![[Foo]], virtuality: DW_VIRTUALITY_virtual, virtualIndex: 2,<br>
// CHECK-SAME: flags: DIFlagPrototyped | DIFlagIntroducedVirtual,<br>
-<br>
-// CHECK: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "Nested",<br>
-// CHECK-SAME: identifier: ".?AUNested@Foo@@"<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>
</div></div></blockquote></div></div></div><br></div>
</blockquote></div><br></div></div>