From 658cd2c287b1a0b419f51cd18e5a48d4560d1c56 Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Sat, 13 Jul 2013 21:08:14 +0000 Subject: [PATCH] PR16214, PR14467: DebugInfo: use "RequireCompleteType" to decide when to emit the full definition of a type in -flimit-debug-info This simplifies the core benefit of -flimit-debug-info by taking a more systematic approach to avoid emitting debug info definitions for types that only require declarations. The previous ad-hoc approach (3 cases removed in this patch) had many holes. The general approach (adding a bit to TagDecl and callback through ASTConsumer) has been discussed with Richard Smith - though always open to revision. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@186262 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/ASTConsumer.h | 4 +++ include/clang/AST/Decl.h | 11 ++++++++ include/clang/Sema/Sema.h | 4 +++ lib/CodeGen/CGClass.cpp | 11 -------- lib/CodeGen/CGDebugInfo.cpp | 3 ++- lib/CodeGen/CGExprCXX.cpp | 11 -------- lib/CodeGen/CGExprScalar.cpp | 12 --------- lib/CodeGen/CodeGenAction.cpp | 4 +++ lib/CodeGen/ModuleBuilder.cpp | 7 +++++ lib/Sema/SemaType.cpp | 15 +++++++++++ test/CodeGenCXX/debug-info-class-limited.cpp | 28 +++++++++++++++----- 11 files changed, 69 insertions(+), 41 deletions(-) diff --git a/include/clang/AST/ASTConsumer.h b/include/clang/AST/ASTConsumer.h index 9109a6741f..7b6fa94b20 100644 --- a/include/clang/AST/ASTConsumer.h +++ b/include/clang/AST/ASTConsumer.h @@ -72,6 +72,10 @@ public: /// can be defined in declspecs). virtual void HandleTagDeclDefinition(TagDecl *D) {} + /// \brief This callback is invoked the first time each TagDecl is required to + /// be complete. + virtual void HandleTagDeclRequiredDefinition(const TagDecl *D) {} + /// \brief Invoked when a function is implicitly instantiated. /// Note that at this point point it does not have a body, its body is /// instantiated at the end of the translation unit and passed to diff --git a/include/clang/AST/Decl.h b/include/clang/AST/Decl.h index 2b2bf1a1ef..cc9a90746b 100644 --- a/include/clang/AST/Decl.h +++ b/include/clang/AST/Decl.h @@ -2445,6 +2445,9 @@ protected: /// This option is only enabled when modules are enabled. bool MayHaveOutOfDateDef : 1; + /// Has the full definition of this type been required by a use somewhere in + /// the TU. + bool IsCompleteDefinitionRequired : 1; private: SourceLocation RBraceLoc; @@ -2531,6 +2534,12 @@ public: return IsCompleteDefinition; } + /// \brief Return true if this complete decl is + /// required to be complete for some existing use. + bool isCompleteDefinitionRequired() const { + return IsCompleteDefinitionRequired; + } + /// isBeingDefined - Return true if this decl is currently being defined. bool isBeingDefined() const { return IsBeingDefined; @@ -2572,6 +2581,8 @@ public: void setCompleteDefinition(bool V) { IsCompleteDefinition = V; } + void setCompleteDefinitionRequired() { IsCompleteDefinitionRequired = true; } + // FIXME: Return StringRef; const char *getKindName() const { return TypeWithKeyword::getTagTypeKindName(getTagKind()); diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index ab392b45c2..6b4079e36e 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -1182,6 +1182,10 @@ public: virtual ~BoundTypeDiagnoser3() { } }; +private: + bool RequireCompleteTypeImpl(SourceLocation Loc, QualType T, + TypeDiagnoser &Diagnoser); +public: bool RequireCompleteType(SourceLocation Loc, QualType T, TypeDiagnoser &Diagnoser); bool RequireCompleteType(SourceLocation Loc, QualType T, diff --git a/lib/CodeGen/CGClass.cpp b/lib/CodeGen/CGClass.cpp index c5220592d5..2bb44552e2 100644 --- a/lib/CodeGen/CGClass.cpp +++ b/lib/CodeGen/CGClass.cpp @@ -1630,17 +1630,6 @@ CodeGenFunction::EmitCXXConstructorCall(const CXXConstructorDecl *D, llvm::Value *This, CallExpr::const_arg_iterator ArgBeg, CallExpr::const_arg_iterator ArgEnd) { - - CGDebugInfo *DI = getDebugInfo(); - if (DI && - CGM.getCodeGenOpts().getDebugInfo() == CodeGenOptions::LimitedDebugInfo) { - // If debug info for this class has not been emitted then this is the - // right time to do so. - const CXXRecordDecl *Parent = D->getParent(); - DI->getOrCreateRecordType(CGM.getContext().getTypeDeclType(Parent), - Parent->getLocation()); - } - // If this is a trivial constructor, just emit what's needed. if (D->isTrivial()) { if (ArgBeg == ArgEnd) { diff --git a/lib/CodeGen/CGDebugInfo.cpp b/lib/CodeGen/CGDebugInfo.cpp index 11149f4792..4c275462d4 100644 --- a/lib/CodeGen/CGDebugInfo.cpp +++ b/lib/CodeGen/CGDebugInfo.cpp @@ -1379,7 +1379,8 @@ llvm::DIType CGDebugInfo::CreateType(const RecordType *Ty, bool Declaration) { RecordDecl *RD = Ty->getDecl(); // Limited debug info should only remove struct definitions that can // safely be replaced by a forward declaration in the source code. - if (DebugKind <= CodeGenOptions::LimitedDebugInfo && Declaration) { + if (DebugKind <= CodeGenOptions::LimitedDebugInfo && Declaration && + !RD->isCompleteDefinitionRequired()) { // FIXME: This implementation is problematic; there are some test // cases where we violate the above principle, such as // test/CodeGen/debug-info-records.c . diff --git a/lib/CodeGen/CGExprCXX.cpp b/lib/CodeGen/CGExprCXX.cpp index 438657ae22..672e86d0d8 100644 --- a/lib/CodeGen/CGExprCXX.cpp +++ b/lib/CodeGen/CGExprCXX.cpp @@ -175,17 +175,6 @@ RValue CodeGenFunction::EmitCXXMemberCallExpr(const CXXMemberCallExpr *CE, const MemberExpr *ME = cast(callee); const CXXMethodDecl *MD = cast(ME->getMemberDecl()); - CGDebugInfo *DI = getDebugInfo(); - if (DI && - CGM.getCodeGenOpts().getDebugInfo() == CodeGenOptions::LimitedDebugInfo && - !isa(ME->getBase())) { - QualType PQTy = ME->getBase()->IgnoreParenImpCasts()->getType(); - if (const PointerType * PTy = dyn_cast(PQTy)) { - DI->getOrCreateRecordType(PTy->getPointeeType(), - MD->getParent()->getLocation()); - } - } - if (MD->isStatic()) { // The method is static, emit it as we would a regular call. llvm::Value *Callee = CGM.GetAddrOfFunction(MD); diff --git a/lib/CodeGen/CGExprScalar.cpp b/lib/CodeGen/CGExprScalar.cpp index f055c67f0c..8e81866665 100644 --- a/lib/CodeGen/CGExprScalar.cpp +++ b/lib/CodeGen/CGExprScalar.cpp @@ -992,18 +992,6 @@ Value *ScalarExprEmitter::VisitMemberExpr(MemberExpr *E) { return Builder.getInt(Value); } - // Emit debug info for aggregate now, if it was delayed to reduce - // debug info size. - CGDebugInfo *DI = CGF.getDebugInfo(); - if (DI && - CGF.CGM.getCodeGenOpts().getDebugInfo() - == CodeGenOptions::LimitedDebugInfo) { - QualType PQTy = E->getBase()->IgnoreParenImpCasts()->getType(); - if (const PointerType * PTy = dyn_cast(PQTy)) - if (FieldDecl *M = dyn_cast(E->getMemberDecl())) - DI->getOrCreateRecordType(PTy->getPointeeType(), - M->getParent()->getLocation()); - } return EmitLoadOfLValue(E); } diff --git a/lib/CodeGen/CodeGenAction.cpp b/lib/CodeGen/CodeGenAction.cpp index 804af31c31..3072204c9b 100644 --- a/lib/CodeGen/CodeGenAction.cpp +++ b/lib/CodeGen/CodeGenAction.cpp @@ -171,6 +171,10 @@ namespace clang { Gen->HandleTagDeclDefinition(D); } + virtual void HandleTagDeclRequiredDefinition(const TagDecl *D) { + Gen->HandleTagDeclRequiredDefinition(D); + } + virtual void CompleteTentativeDefinition(VarDecl *D) { Gen->CompleteTentativeDefinition(D); } diff --git a/lib/CodeGen/ModuleBuilder.cpp b/lib/CodeGen/ModuleBuilder.cpp index 763aa12aab..7e0e3aac07 100644 --- a/lib/CodeGen/ModuleBuilder.cpp +++ b/lib/CodeGen/ModuleBuilder.cpp @@ -13,6 +13,7 @@ #include "clang/CodeGen/ModuleBuilder.h" #include "CodeGenModule.h" +#include "CGDebugInfo.h" #include "clang/AST/ASTContext.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/Expr.h" @@ -93,6 +94,12 @@ namespace { } } + virtual void HandleTagDeclRequiredDefinition(const TagDecl *D) LLVM_OVERRIDE { + if (CodeGen::CGDebugInfo *DI = Builder->getModuleDebugInfo()) + if (const RecordDecl *RD = dyn_cast(D)) + DI->completeFwdDecl(*RD); + } + virtual void HandleTranslationUnit(ASTContext &Ctx) { if (Diags.hasErrorOccurred()) { M.reset(); diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp index 72a6d1aaf9..80e9b2fba0 100644 --- a/lib/Sema/SemaType.cpp +++ b/lib/Sema/SemaType.cpp @@ -12,6 +12,7 @@ //===----------------------------------------------------------------------===// #include "clang/Sema/SemaInternal.h" +#include "clang/AST/ASTConsumer.h" #include "clang/AST/ASTContext.h" #include "clang/AST/ASTMutationListener.h" #include "clang/AST/CXXInheritance.h" @@ -4844,6 +4845,20 @@ bool Sema::RequireCompleteExprType(Expr *E, unsigned DiagID) { /// @c false otherwise. bool Sema::RequireCompleteType(SourceLocation Loc, QualType T, TypeDiagnoser &Diagnoser) { + if (RequireCompleteTypeImpl(Loc, T, Diagnoser)) + return true; + if (const TagType *Tag = T->getAs()) { + if (!Tag->getDecl()->isCompleteDefinitionRequired()) { + Tag->getDecl()->setCompleteDefinitionRequired(); + Consumer.HandleTagDeclRequiredDefinition(Tag->getDecl()); + } + } + return false; +} + +/// \brief The implementation of RequireCompleteType +bool Sema::RequireCompleteTypeImpl(SourceLocation Loc, QualType T, + TypeDiagnoser &Diagnoser) { // FIXME: Add this assertion to make sure we always get instantiation points. // assert(!Loc.isInvalid() && "Invalid location in RequireCompleteType"); // FIXME: Add this assertion to help us flush out problems with diff --git a/test/CodeGenCXX/debug-info-class-limited.cpp b/test/CodeGenCXX/debug-info-class-limited.cpp index 26ee19f2f4..a4b9f46ec6 100644 --- a/test/CodeGenCXX/debug-info-class-limited.cpp +++ b/test/CodeGenCXX/debug-info-class-limited.cpp @@ -1,8 +1,7 @@ // RUN: %clang -emit-llvm -g -S %s -o - | FileCheck %s namespace PR16214_1 { -// CHECK: [[PR16214_1:![0-9]*]] = {{.*}} [ DW_TAG_namespace ] [PR16214_1] -// CHECK: = metadata !{i32 {{[0-9]*}}, metadata !{{[0-9]*}}, metadata [[PR16214_1]], {{.*}} ; [ DW_TAG_structure_type ] [foo] {{.*}} [def] +// CHECK-DAG: [ DW_TAG_structure_type ] [foo] [line [[@LINE+1]], {{.*}} [def] struct foo { int i; }; @@ -13,9 +12,9 @@ bar *a; bar b; } -namespace test1 { +namespace PR14467 { +// CHECK-DAG: [ DW_TAG_structure_type ] [foo] [line [[@LINE+1]], {{.*}} [def] struct foo { - int i; }; foo *bar(foo *a) { @@ -24,9 +23,9 @@ foo *bar(foo *a) { } } -namespace test2 { +namespace test1 { +// CHECK-DAG: [ DW_TAG_structure_type ] [foo] [line [[@LINE+1]], {{.*}} [decl] struct foo { - int i; }; extern int bar(foo *a); @@ -34,3 +33,20 @@ int baz(foo *a) { return bar(a); } } + +namespace test2 { +// FIXME: if we were a bit fancier, we could realize that the 'foo' type is only +// required because of the 'bar' type which is not required at all (or might +// only be required to be declared) +// CHECK-DAG: [ DW_TAG_structure_type ] [foo] [line [[@LINE+1]], {{.*}} [def] +struct foo { +}; + +struct bar { + foo f; +}; + +void func() { + foo *f; +} +} -- 2.40.0