<div dir="ltr">Thanks!</div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jan 8, 2016 at 5:15 PM, Adrian Prantl <span dir="ltr"><<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
> On Jan 8, 2016, at 2:18 PM, Adrian Prantl via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br>
><br>
>><br>
>> On Jan 8, 2016, at 1:59 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
>><br>
>><br>
>><br>
>> On Fri, Jan 8, 2016 at 1:34 PM, Adrian Prantl <<a href="mailto:aprantl@apple.com">aprantl@apple.com</a>> wrote:<br>
>><br>
>>> On Jan 8, 2016, at 1:31 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
>>><br>
>>><br>
>>><br>
>>> On Wed, Jan 6, 2016 at 11:22 AM, Adrian Prantl via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br>
>>> Author: adrian<br>
>>> Date: Wed Jan 6 13:22:19 2016<br>
>>> New Revision: 256962<br>
>>><br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=256962&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=256962&view=rev</a><br>
>>> Log:<br>
>>> Module debugging: Defer emitting tag types until their definition<br>
>>> was visited and all decls have been merged.<br>
>>><br>
>>> We only get a single chance to emit the types for virtual classes because<br>
>>> CGDebugInfo::completeRequiredType() categorically doesn't complete them.<br>
>>><br>
>>> Not sure I'm following this comment. Could you explain this in other words/detail?<br>
>>><br>
>>> If we visit a declaration we shouldn't do anything, right? (no point putting declarations in the modules debug info, unless it's needed for something else?) & then when we see the definition we'd build the debug info for it?<br>
>><br>
>> Yes. Your statement pretty much summarizes this commit :-)<br>
>><br>
>> Then I'm still confused - were we emitting type declarations in modules prior to this change?<br>
><br>
> Yes. Dsymutil used to expect every forward declaration in a skeleton CU to have a corresponding declaration in the module dwo. I just verified that dsymutil does no longer prune forward declarations from skeleton CUs that don’t have a definition in the DWO, so everything should be fine now.<br>
><br>
>> Perhaps a test case for a lone declaration (with no follow up definition) would be in order, then?<br>
><br>
> Yes, that would make sense. I’ll add one.<br>
<br>
</span>Done in r257241.<br>
<span class="HOEnZb"><font color="#888888">-- adrian<br>
</font></span><div class="HOEnZb"><div class="h5">><br>
>><br>
>> -- adrian<br>
>><br>
>>><br>
>>><br>
>>> Modified:<br>
>>> cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp<br>
>>> cfe/trunk/test/Modules/Inputs/DebugCXX.h<br>
>>> cfe/trunk/test/Modules/ModuleDebugInfo.cpp<br>
>>><br>
>>> Modified: cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp<br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp?rev=256962&r1=256961&r2=256962&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp?rev=256962&r1=256961&r2=256962&view=diff</a><br>
>>> ==============================================================================<br>
>>> --- cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp (original)<br>
>>> +++ cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp Wed Jan 6 13:22:19 2016<br>
>>> @@ -59,8 +59,10 @@ class PCHContainerGenerator : public AST<br>
>>> struct DebugTypeVisitor : public RecursiveASTVisitor<DebugTypeVisitor> {<br>
>>> clang::CodeGen::CGDebugInfo &DI;<br>
>>> ASTContext &Ctx;<br>
>>> - DebugTypeVisitor(clang::CodeGen::CGDebugInfo &DI, ASTContext &Ctx)<br>
>>> - : DI(DI), Ctx(Ctx) {}<br>
>>> + bool SkipTagDecls;<br>
>>> + DebugTypeVisitor(clang::CodeGen::CGDebugInfo &DI, ASTContext &Ctx,<br>
>>> + bool SkipTagDecls)<br>
>>> + : DI(DI), Ctx(Ctx), SkipTagDecls(SkipTagDecls) {}<br>
>>><br>
>>> /// Determine whether this type can be represented in DWARF.<br>
>>> static bool CanRepresent(const Type *Ty) {<br>
>>> @@ -75,6 +77,12 @@ class PCHContainerGenerator : public AST<br>
>>> }<br>
>>><br>
>>> bool VisitTypeDecl(TypeDecl *D) {<br>
>>> + // TagDecls may be deferred until after all decls have been merged and we<br>
>>> + // know the complete type. Pure forward declarations will be skipped, but<br>
>>> + // they don't need to be emitted into the module anyway.<br>
>>> + if (SkipTagDecls && isa<TagDecl>(D))<br>
>>> + return true;<br>
>>> +<br>
>>> QualType QualTy = Ctx.getTypeDeclType(D);<br>
>>> if (!QualTy.isNull() && CanRepresent(QualTy.getTypePtr()))<br>
>>> DI.getOrCreateStandaloneType(QualTy, D->getLocation());<br>
>>> @@ -165,7 +173,7 @@ public:<br>
>>> // Collect debug info for all decls in this group.<br>
>>> for (auto *I : D)<br>
>>> if (!I->isFromASTFile()) {<br>
>>> - DebugTypeVisitor DTV(*Builder->getModuleDebugInfo(), *Ctx);<br>
>>> + DebugTypeVisitor DTV(*Builder->getModuleDebugInfo(), *Ctx, true);<br>
>>> DTV.TraverseDecl(I);<br>
>>> }<br>
>>> return true;<br>
>>> @@ -179,6 +187,11 @@ public:<br>
>>> if (Diags.hasErrorOccurred())<br>
>>> return;<br>
>>><br>
>>> + if (D->isFromASTFile())<br>
>>> + return;<br>
>>> +<br>
>>> + DebugTypeVisitor DTV(*Builder->getModuleDebugInfo(), *Ctx, false);<br>
>>> + DTV.TraverseDecl(D);<br>
>>> Builder->UpdateCompletedType(D);<br>
>>> }<br>
>>><br>
>>><br>
>>> Modified: cfe/trunk/test/Modules/Inputs/DebugCXX.h<br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/DebugCXX.h?rev=256962&r1=256961&r2=256962&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/DebugCXX.h?rev=256962&r1=256961&r2=256962&view=diff</a><br>
>>> ==============================================================================<br>
>>> --- cfe/trunk/test/Modules/Inputs/DebugCXX.h (original)<br>
>>> +++ cfe/trunk/test/Modules/Inputs/DebugCXX.h Wed Jan 6 13:22:19 2016<br>
>>> @@ -50,3 +50,9 @@ namespace DebugCXX {<br>
>>> typedef A<void> B;<br>
>>> void foo(B) {}<br>
>>> }<br>
>>> +<br>
>>> +// Virtual class with a forward declaration.<br>
>>> +class FwdVirtual;<br>
>>> +class FwdVirtual {<br>
>>> + virtual ~FwdVirtual() {}<br>
>>> +};<br>
>>><br>
>>> Modified: cfe/trunk/test/Modules/ModuleDebugInfo.cpp<br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ModuleDebugInfo.cpp?rev=256962&r1=256961&r2=256962&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ModuleDebugInfo.cpp?rev=256962&r1=256961&r2=256962&view=diff</a><br>
>>> ==============================================================================<br>
>>> --- cfe/trunk/test/Modules/ModuleDebugInfo.cpp (original)<br>
>>> +++ cfe/trunk/test/Modules/ModuleDebugInfo.cpp Wed Jan 6 13:22:19 2016<br>
>>> @@ -7,13 +7,11 @@<br>
>>> // RUN: rm -rf %t<br>
>>> // RUN: %clang_cc1 -triple %itanium_abi_triple -x objective-c++ -std=c++11 -debug-info-kind=limited -fmodules -fmodule-format=obj -fimplicit-module-maps -DMODULES -fmodules-cache-path=%t %s -I %S/Inputs -I %t -emit-llvm -o %t.ll -mllvm -debug-only=pchcontainer &>%t-mod.ll<br>
>>> // RUN: cat %t-mod.ll | FileCheck %s<br>
>>> -// RUN: cat %t-mod.ll | FileCheck --check-prefix=CHECK-NEG %s<br>
>>> // RUN: cat %t-mod.ll | FileCheck --check-prefix=CHECK-DWO %s<br>
>>><br>
>>> // PCH:<br>
>>> // RUN: %clang_cc1 -triple %itanium_abi_triple -x c++ -std=c++11 -emit-pch -fmodule-format=obj -I %S/Inputs -o %t.pch %S/Inputs/DebugCXX.h -mllvm -debug-only=pchcontainer &>%t-pch.ll<br>
>>> // RUN: cat %t-pch.ll | FileCheck %s<br>
>>> -// RUN: cat %t-pch.ll | FileCheck --check-prefix=CHECK-NEG %s<br>
>>><br>
>>> #ifdef MODULES<br>
>>> @import DebugCXX;<br>
>>> @@ -23,22 +21,32 @@<br>
>>> // CHECK-SAME: isOptimized: false,<br>
>>> // CHECK-SAME-NOT: splitDebugFilename:<br>
>>> // CHECK-DWO: dwoId:<br>
>>> +<br>
>>> // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "Enum"<br>
>>> // CHECK-SAME: identifier: "_ZTSN8DebugCXX4EnumE")<br>
>>> // CHECK: !DINamespace(name: "DebugCXX"<br>
>>> +<br>
>>> // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "Struct"<br>
>>> // CHECK-SAME: identifier: "_ZTSN8DebugCXX6StructE")<br>
>>> +<br>
>>> // CHECK: !DICompositeType(tag: DW_TAG_class_type,<br>
>>> // CHECK-SAME: name: "Template<int, DebugCXX::traits<int> >"<br>
>>> // CHECK-SAME: identifier: "_ZTSN8DebugCXX8TemplateIiNS_6traitsIiEEEE")<br>
>>> +<br>
>>> +// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "A<void>"<br>
>>> +// CHECK-SAME: identifier: "_ZTSN8DebugCXX1AIJvEEE")<br>
>>> +<br>
>>> // CHECK: !DICompositeType(tag: DW_TAG_class_type,<br>
>>> // CHECK-SAME: name: "Template<float, DebugCXX::traits<float> >"<br>
>>> // CHECK-SAME: identifier: "_ZTSN8DebugCXX8TemplateIfNS_6traitsIfEEEE")<br>
>>> -// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "A<void>"<br>
>>> -// CHECK-SAME: identifier: "_ZTSN8DebugCXX1AIJvEEE")<br>
>>> +<br>
>>> +// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "FwdVirtual"<br>
>>> +// CHECK-SAME: elements:<br>
>>> +// CHECK-SAME: identifier: "_ZTS10FwdVirtual")<br>
>>> +// CHECK: !DIDerivedType(tag: DW_TAG_member, name: "_vptr$FwdVirtual"<br>
>>> +<br>
>>> // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "FloatInstatiation"<br>
>>> // no mangled name here yet.<br>
>>> +<br>
>>> // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "B",<br>
>>> // no mangled name here yet.<br>
>>> -<br>
>>> -// CHECK-NEG-NOT: "_ZTSN8DebugCXX8TemplateIlNS_6traitsIlEEEE"<br>
>>><br>
>>><br>
>>> _______________________________________________<br>
>>> cfe-commits mailing list<br>
>>> <a href="mailto:cfe-commits@lists.llvm.org">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>
>>><br>
>><br>
>><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@lists.llvm.org">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>
<br>
</div></div></blockquote></div><br></div>