dblaikie added inline comments.
================ Comment at: clang/lib/CodeGen/CGDecl.cpp:103-118 + case Decl::CXXRecord: // struct/union/class X; [C++] + if (CGDebugInfo *DI = getDebugInfo()) + if (CGM.getCodeGenOpts().hasMaybeUnusedDebugInfo() && + cast<CXXRecordDecl>(D).hasDefinition()) + DI->completeUnusedClass(cast<CXXRecordDecl>(D)); + return; case Decl::Record: // struct/union/class X; ---------------- All of these might be able to be collapsed together - using "cast<TagDecl>(D).hasDefinition()" ... to have a common implementation. ================ Comment at: clang/lib/CodeGen/CGDecl.cpp:105-107 + if (CGM.getCodeGenOpts().hasMaybeUnusedDebugInfo() && + cast<CXXRecordDecl>(D).hasDefinition()) + DI->completeUnusedClass(cast<CXXRecordDecl>(D)); ---------------- Any particular reason this one's handled differently from the others? ================ Comment at: clang/lib/CodeGen/CGDecl.cpp:116 + if (CGDebugInfo *DI = getDebugInfo()) + if (cast<EnumDecl>(&D)->getDefinition()) + DI->EmitAndRetainType(getContext().getEnumType(cast<EnumDecl>(&D))); ---------------- I think the right thing to call here (& the other places in this patch) is "hasDefinition" rather than "getDefinition" (this will avoid the definition being deserialized when it's not needed yet (then if we don't end up emitting the type because the flag hasn't been given, it won't be deserialized unnecessarily)) ================ Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5385 + if (getCodeGenOpts().hasMaybeUnusedDebugInfo() && CRD->hasDefinition()) + DI->completeUnusedClass(*CRD); + else if (auto *ES = D->getASTContext().getExternalSource()) ---------------- I think this might not work as intended if the type trips over the other class deduplication optimizations (try manually testing with a type that has an external VTable?) - and perhaps this codepath should use the EmitAndRetain functionality. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80242/new/ https://reviews.llvm.org/D80242 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
