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.

llvm-svn: 186262

clang/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

clang/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());

clang/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,

clang/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) {

clang/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 .

clang/lib/CodeGen/CGExprCXX.cpp

@@ -175,17 +175,6 @@ RValue CodeGenFunction::EmitCXXMemberCallExpr(const CXXMemberCallExpr *CE,
   const MemberExpr *ME = cast<MemberExpr>(callee);
   const CXXMethodDecl *MD = cast<CXXMethodDecl>(ME->getMemberDecl());
 
-  CGDebugInfo *DI = getDebugInfo();
-  if (DI &&
-      CGM.getCodeGenOpts().getDebugInfo() == CodeGenOptions::LimitedDebugInfo &&
-      !isa<CallExpr>(ME->getBase())) {
-    QualType PQTy = ME->getBase()->IgnoreParenImpCasts()->getType();
-    if (const PointerType * PTy = dyn_cast<PointerType>(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);

clang/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<PointerType>(PQTy))
-      if (FieldDecl *M = dyn_cast<FieldDecl>(E->getMemberDecl()))
-        DI->getOrCreateRecordType(PTy->getPointeeType(),
-                                  M->getParent()->getLocation());
-  }
   return EmitLoadOfLValue(E);
 }
 

clang/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);
     }

clang/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<RecordDecl>(D))
+          DI->completeFwdDecl(*RD);
+    }
+
     virtual void HandleTranslationUnit(ASTContext &Ctx) {
       if (Diags.hasErrorOccurred()) {
         M.reset();

clang/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<TagType>()) {
+    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

clang/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;
+}
+}