> On Jan 8, 2016, at 1:59 PM, David Blaikie <dblai...@gmail.com> wrote: > > > > On Fri, Jan 8, 2016 at 1:34 PM, Adrian Prantl <apra...@apple.com > <mailto:apra...@apple.com>> wrote: > >> On Jan 8, 2016, at 1:31 PM, David Blaikie <dblai...@gmail.com >> <mailto:dblai...@gmail.com>> wrote: >> >> >> >> On Wed, Jan 6, 2016 at 11:22 AM, Adrian Prantl via cfe-commits >> <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: >> Author: adrian >> Date: Wed Jan 6 13:22:19 2016 >> New Revision: 256962 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=256962&view=rev >> <http://llvm.org/viewvc/llvm-project?rev=256962&view=rev> >> Log: >> Module debugging: Defer emitting tag types until their definition >> was visited and all decls have been merged. >> >> We only get a single chance to emit the types for virtual classes because >> CGDebugInfo::completeRequiredType() categorically doesn't complete them. >> >> Not sure I'm following this comment. Could you explain this in other >> words/detail? >> >> 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? > > Yes. Your statement pretty much summarizes this commit :-) > > Then I'm still confused - were we emitting type declarations in modules prior > to this change?
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. > Perhaps a test case for a lone declaration (with no follow up definition) > would be in order, then? Yes, that would make sense. I’ll add one. > > -- adrian > >> >> >> Modified: >> cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp >> cfe/trunk/test/Modules/Inputs/DebugCXX.h >> cfe/trunk/test/Modules/ModuleDebugInfo.cpp >> >> Modified: cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp?rev=256962&r1=256961&r2=256962&view=diff >> >> <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp?rev=256962&r1=256961&r2=256962&view=diff> >> ============================================================================== >> --- cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp (original) >> +++ cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp Wed Jan 6 >> 13:22:19 2016 >> @@ -59,8 +59,10 @@ class PCHContainerGenerator : public AST >> struct DebugTypeVisitor : public RecursiveASTVisitor<DebugTypeVisitor> { >> clang::CodeGen::CGDebugInfo &DI; >> ASTContext &Ctx; >> - DebugTypeVisitor(clang::CodeGen::CGDebugInfo &DI, ASTContext &Ctx) >> - : DI(DI), Ctx(Ctx) {} >> + bool SkipTagDecls; >> + DebugTypeVisitor(clang::CodeGen::CGDebugInfo &DI, ASTContext &Ctx, >> + bool SkipTagDecls) >> + : DI(DI), Ctx(Ctx), SkipTagDecls(SkipTagDecls) {} >> >> /// Determine whether this type can be represented in DWARF. >> static bool CanRepresent(const Type *Ty) { >> @@ -75,6 +77,12 @@ class PCHContainerGenerator : public AST >> } >> >> bool VisitTypeDecl(TypeDecl *D) { >> + // TagDecls may be deferred until after all decls have been merged >> and we >> + // know the complete type. Pure forward declarations will be skipped, >> but >> + // they don't need to be emitted into the module anyway. >> + if (SkipTagDecls && isa<TagDecl>(D)) >> + return true; >> + >> QualType QualTy = Ctx.getTypeDeclType(D); >> if (!QualTy.isNull() && CanRepresent(QualTy.getTypePtr())) >> DI.getOrCreateStandaloneType(QualTy, D->getLocation()); >> @@ -165,7 +173,7 @@ public: >> // Collect debug info for all decls in this group. >> for (auto *I : D) >> if (!I->isFromASTFile()) { >> - DebugTypeVisitor DTV(*Builder->getModuleDebugInfo(), *Ctx); >> + DebugTypeVisitor DTV(*Builder->getModuleDebugInfo(), *Ctx, true); >> DTV.TraverseDecl(I); >> } >> return true; >> @@ -179,6 +187,11 @@ public: >> if (Diags.hasErrorOccurred()) >> return; >> >> + if (D->isFromASTFile()) >> + return; >> + >> + DebugTypeVisitor DTV(*Builder->getModuleDebugInfo(), *Ctx, false); >> + DTV.TraverseDecl(D); >> Builder->UpdateCompletedType(D); >> } >> >> >> Modified: cfe/trunk/test/Modules/Inputs/DebugCXX.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/DebugCXX.h?rev=256962&r1=256961&r2=256962&view=diff >> >> <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/DebugCXX.h?rev=256962&r1=256961&r2=256962&view=diff> >> ============================================================================== >> --- cfe/trunk/test/Modules/Inputs/DebugCXX.h (original) >> +++ cfe/trunk/test/Modules/Inputs/DebugCXX.h Wed Jan 6 13:22:19 2016 >> @@ -50,3 +50,9 @@ namespace DebugCXX { >> typedef A<void> B; >> void foo(B) {} >> } >> + >> +// Virtual class with a forward declaration. >> +class FwdVirtual; >> +class FwdVirtual { >> + virtual ~FwdVirtual() {} >> +}; >> >> Modified: cfe/trunk/test/Modules/ModuleDebugInfo.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ModuleDebugInfo.cpp?rev=256962&r1=256961&r2=256962&view=diff >> >> <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ModuleDebugInfo.cpp?rev=256962&r1=256961&r2=256962&view=diff> >> ============================================================================== >> --- cfe/trunk/test/Modules/ModuleDebugInfo.cpp (original) >> +++ cfe/trunk/test/Modules/ModuleDebugInfo.cpp Wed Jan 6 13:22:19 2016 >> @@ -7,13 +7,11 @@ >> // RUN: rm -rf %t >> // 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 >> // RUN: cat %t-mod.ll | FileCheck %s >> -// RUN: cat %t-mod.ll | FileCheck --check-prefix=CHECK-NEG %s >> // RUN: cat %t-mod.ll | FileCheck --check-prefix=CHECK-DWO %s >> >> // PCH: >> // 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 >> // RUN: cat %t-pch.ll | FileCheck %s >> -// RUN: cat %t-pch.ll | FileCheck --check-prefix=CHECK-NEG %s >> >> #ifdef MODULES >> @import DebugCXX; >> @@ -23,22 +21,32 @@ >> // CHECK-SAME: isOptimized: false, >> // CHECK-SAME-NOT: splitDebugFilename: >> // CHECK-DWO: dwoId: >> + >> // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "Enum" >> // CHECK-SAME: identifier: "_ZTSN8DebugCXX4EnumE") >> // CHECK: !DINamespace(name: "DebugCXX" >> + >> // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "Struct" >> // CHECK-SAME: identifier: "_ZTSN8DebugCXX6StructE") >> + >> // CHECK: !DICompositeType(tag: DW_TAG_class_type, >> // CHECK-SAME: name: "Template<int, DebugCXX::traits<int> >" >> // CHECK-SAME: identifier: >> "_ZTSN8DebugCXX8TemplateIiNS_6traitsIiEEEE") >> + >> +// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "A<void>" >> +// CHECK-SAME: identifier: "_ZTSN8DebugCXX1AIJvEEE") >> + >> // CHECK: !DICompositeType(tag: DW_TAG_class_type, >> // CHECK-SAME: name: "Template<float, DebugCXX::traits<float> >" >> // CHECK-SAME: identifier: >> "_ZTSN8DebugCXX8TemplateIfNS_6traitsIfEEEE") >> -// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "A<void>" >> -// CHECK-SAME: identifier: "_ZTSN8DebugCXX1AIJvEEE") >> + >> +// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "FwdVirtual" >> +// CHECK-SAME: elements: >> +// CHECK-SAME: identifier: "_ZTS10FwdVirtual") >> +// CHECK: !DIDerivedType(tag: DW_TAG_member, name: "_vptr$FwdVirtual" >> + >> // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "FloatInstatiation" >> // no mangled name here yet. >> + >> // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "B", >> // no mangled name here yet. >> - >> -// CHECK-NEG-NOT: "_ZTSN8DebugCXX8TemplateIlNS_6traitsIlEEEE" >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits> >> > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
