> On Jan 8, 2016, at 2:18 PM, Adrian Prantl via cfe-commits > <cfe-commits@lists.llvm.org> wrote: > >> >> 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> wrote: >> >>> On Jan 8, 2016, at 1:31 PM, David Blaikie <dblai...@gmail.com> wrote: >>> >>> >>> >>> On Wed, Jan 6, 2016 at 11:22 AM, Adrian Prantl via cfe-commits >>> <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 >>> 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.
Done in r257241. -- adrian > >> >> -- 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 >>> ============================================================================== >>> --- 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 >>> ============================================================================== >>> --- 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 >>> ============================================================================== >>> --- 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 >>> 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
