Thanks! On Fri, Jan 8, 2016 at 5:15 PM, Adrian Prantl <apra...@apple.com> wrote:
> > > 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
