Extract a common structure for holding information about the definition

of a C++ record.  Exposed a lot of problems where various routines were
silently doing The Wrong Thing (or The Acceptable Thing in The Wrong Order)
when presented with a non-definition.  Also cuts down on memory usage.



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@95330 91177308-0d34-0410-b5e6-96231b3b80d8

9 files changed, 124 insertions, 70 deletions

diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
index c3e976a080..ba1def643e 100644
--- a/lib/AST/Decl.cpp
+++ b/lib/AST/Decl.cpp
@@ -1373,6 +1373,16 @@ void TagDecl::startDefinition() {
     TagT->decl.setPointer(this);
     TagT->decl.setInt(1);
   }
+
+  if (isa<CXXRecordDecl>(this)) {
+    CXXRecordDecl *D = cast<CXXRecordDecl>(this);
+    struct CXXRecordDecl::DefinitionData *Data = 
+      new (getASTContext()) struct CXXRecordDecl::DefinitionData(D);
+    do {
+      D->DefinitionData = Data;
+      D = cast_or_null<CXXRecordDecl>(D->getPreviousDeclaration());
+    } while (D);
+  }
 }
 
 void TagDecl::completeDefinition() {
diff --git a/lib/AST/DeclCXX.cpp b/lib/AST/DeclCXX.cpp
index 3d690d15f2..02ad36d479 100644
--- a/lib/AST/DeclCXX.cpp
+++ b/lib/AST/DeclCXX.cpp
@@ -25,18 +25,23 @@ using namespace clang;
 // Decl Allocation/Deallocation Method Implementations
 //===----------------------------------------------------------------------===//
 
-CXXRecordDecl::CXXRecordDecl(Kind K, TagKind TK, DeclContext *DC,
-                             SourceLocation L, IdentifierInfo *Id,
-                             CXXRecordDecl *PrevDecl,
-                             SourceLocation TKL)
-  : RecordDecl(K, TK, DC, L, Id, PrevDecl, TKL),
-    UserDeclaredConstructor(false), UserDeclaredCopyConstructor(false),
+CXXRecordDecl::DefinitionData::DefinitionData(CXXRecordDecl *D)
+  : UserDeclaredConstructor(false), UserDeclaredCopyConstructor(false),
     UserDeclaredCopyAssignment(false), UserDeclaredDestructor(false),
     Aggregate(true), PlainOldData(true), Empty(true), Polymorphic(false),
     Abstract(false), HasTrivialConstructor(true),
     HasTrivialCopyConstructor(true), HasTrivialCopyAssignment(true),
     HasTrivialDestructor(true), ComputedVisibleConversions(false),
     Bases(0), NumBases(0), VBases(0), NumVBases(0),
+    Definition(D) {
+}
+
+CXXRecordDecl::CXXRecordDecl(Kind K, TagKind TK, DeclContext *DC,
+                             SourceLocation L, IdentifierInfo *Id,
+                             CXXRecordDecl *PrevDecl,
+                             SourceLocation TKL)
+  : RecordDecl(K, TK, DC, L, Id, PrevDecl, TKL),
+    DefinitionData(PrevDecl ? PrevDecl->DefinitionData : 0),
     TemplateOrInstantiation() { }
 
 CXXRecordDecl *CXXRecordDecl::Create(ASTContext &C, TagKind TK, DeclContext *DC,
@@ -57,8 +62,11 @@ CXXRecordDecl::~CXXRecordDecl() {
 }
 
 void CXXRecordDecl::Destroy(ASTContext &C) {
-  C.Deallocate(Bases);
-  C.Deallocate(VBases);
+  if (data().Definition == this) {
+    C.Deallocate(data().Bases);
+    C.Deallocate(data().VBases);
+    C.Deallocate(&data());
+  }
   this->RecordDecl::Destroy(C);
 }
 
@@ -69,19 +77,19 @@ CXXRecordDecl::setBases(ASTContext &C,
   // C++ [dcl.init.aggr]p1:
   //   An aggregate is an array or a class (clause 9) with [...]
   //   no base classes [...].
-  Aggregate = false;
+  data().Aggregate = false;
 
-  if (this->Bases)
-    C.Deallocate(this->Bases);
+  if (data().Bases)
+    C.Deallocate(data().Bases);
 
   int vbaseCount = 0;
   llvm::SmallVector<const CXXBaseSpecifier*, 8> UniqueVbases;
   bool hasDirectVirtualBase = false;
 
-  this->Bases = new(C) CXXBaseSpecifier [NumBases];
-  this->NumBases = NumBases;
+  data().Bases = new(C) CXXBaseSpecifier [NumBases];
+  data().NumBases = NumBases;
   for (unsigned i = 0; i < NumBases; ++i) {
-    this->Bases[i] = *Bases[i];
+    data().Bases[i] = *Bases[i];
     // Keep track of inherited vbases for this base class.
     const CXXBaseSpecifier *Base = Bases[i];
     QualType BaseType = Base->getType();
@@ -130,13 +138,13 @@ CXXRecordDecl::setBases(ASTContext &C,
   }
   if (vbaseCount > 0) {
     // build AST for inhireted, direct or indirect, virtual bases.
-    this->VBases = new (C) CXXBaseSpecifier [vbaseCount];
-    this->NumVBases = vbaseCount;
+    data().VBases = new (C) CXXBaseSpecifier [vbaseCount];
+    data().NumVBases = vbaseCount;
     for (int i = 0; i < vbaseCount; i++) {
       QualType QT = UniqueVbases[i]->getType();
       CXXRecordDecl *VBaseClassDecl
         = cast<CXXRecordDecl>(QT->getAs<RecordType>()->getDecl());
-      this->VBases[i] =
+      data().VBases[i] =
         CXXBaseSpecifier(VBaseClassDecl->getSourceRange(), true,
                          VBaseClassDecl->getTagKind() == RecordDecl::TK_class,
                          UniqueVbases[i]->getAccessSpecifier(), QT);
@@ -239,32 +247,32 @@ CXXRecordDecl::addedConstructor(ASTContext &Context,
                                 CXXConstructorDecl *ConDecl) {
   assert(!ConDecl->isImplicit() && "addedConstructor - not for implicit decl");
   // Note that we have a user-declared constructor.
-  UserDeclaredConstructor = true;
+  data().UserDeclaredConstructor = true;
 
   // C++ [dcl.init.aggr]p1:
   //   An aggregate is an array or a class (clause 9) with no
   //   user-declared constructors (12.1) [...].
-  Aggregate = false;
+  data().Aggregate = false;
 
   // C++ [class]p4:
   //   A POD-struct is an aggregate class [...]
-  PlainOldData = false;
+  data().PlainOldData = false;
 
   // C++ [class.ctor]p5:
   //   A constructor is trivial if it is an implicitly-declared default
   //   constructor.
   // FIXME: C++0x: don't do this for "= default" default constructors.
-  HasTrivialConstructor = false;
+  data().HasTrivialConstructor = false;
 
   // Note when we have a user-declared copy constructor, which will
   // suppress the implicit declaration of a copy constructor.
   if (ConDecl->isCopyConstructor()) {
-    UserDeclaredCopyConstructor = true;
+    data().UserDeclaredCopyConstructor = true;
 
     // C++ [class.copy]p6:
     //   A copy constructor is trivial if it is implicitly declared.
     // FIXME: C++0x: don't do this for "= default" copy constructors.
-    HasTrivialCopyConstructor = false;
+    data().HasTrivialCopyConstructor = false;
   }
 }
 
@@ -295,17 +303,17 @@ void CXXRecordDecl::addedAssignmentOperator(ASTContext &Context,
   OpDecl->setCopyAssignment(true);
 
   // Suppress the implicit declaration of a copy constructor.
-  UserDeclaredCopyAssignment = true;
+  data().UserDeclaredCopyAssignment = true;
 
   // C++ [class.copy]p11:
   //   A copy assignment operator is trivial if it is implicitly declared.
   // FIXME: C++0x: don't do this for "= default" copy operators.
-  HasTrivialCopyAssignment = false;
+  data().HasTrivialCopyAssignment = false;
 
   // C++ [class]p4:
   //   A POD-struct is an aggregate class that [...] has no user-defined copy
   //   assignment operator [...].
-  PlainOldData = false;
+  data().PlainOldData = false;
 }
 
 void
@@ -415,42 +423,42 @@ CXXRecordDecl::getNestedVisibleConversionFunctions(CXXRecordDecl *RD,
 const UnresolvedSetImpl *CXXRecordDecl::getVisibleConversionFunctions() {
   // If root class, all conversions are visible.
   if (bases_begin() == bases_end())
-    return &Conversions;
+    return &data().Conversions;
   // If visible conversion list is already evaluated, return it.
-  if (ComputedVisibleConversions)
-    return &VisibleConversions;
+  if (data().ComputedVisibleConversions)
+    return &data().VisibleConversions;
   llvm::SmallPtrSet<CanQualType, 8> TopConversionsTypeSet;
   collectConversionFunctions(TopConversionsTypeSet);
   getNestedVisibleConversionFunctions(this, TopConversionsTypeSet,
                                       TopConversionsTypeSet);
-  ComputedVisibleConversions = true;
-  return &VisibleConversions;
+  data().ComputedVisibleConversions = true;
+  return &data().VisibleConversions;
 }
 
 void CXXRecordDecl::addVisibleConversionFunction(
                                           CXXConversionDecl *ConvDecl) {
   assert(!ConvDecl->getDescribedFunctionTemplate() &&
          "Conversion function templates should cast to FunctionTemplateDecl.");
-  VisibleConversions.addDecl(ConvDecl);
+  data().VisibleConversions.addDecl(ConvDecl);
 }
 
 void CXXRecordDecl::addVisibleConversionFunction(
                                           FunctionTemplateDecl *ConvDecl) {
   assert(isa<CXXConversionDecl>(ConvDecl->getTemplatedDecl()) &&
          "Function template is not a conversion function template");
-  VisibleConversions.addDecl(ConvDecl);
+  data().VisibleConversions.addDecl(ConvDecl);
 }
 
 void CXXRecordDecl::addConversionFunction(CXXConversionDecl *ConvDecl) {
   assert(!ConvDecl->getDescribedFunctionTemplate() &&
          "Conversion function templates should cast to FunctionTemplateDecl.");
-  Conversions.addDecl(ConvDecl);
+  data().Conversions.addDecl(ConvDecl);
 }
 
 void CXXRecordDecl::addConversionFunction(FunctionTemplateDecl *ConvDecl) {
   assert(isa<CXXConversionDecl>(ConvDecl->getTemplatedDecl()) &&
          "Function template is not a conversion function template");
-  Conversions.addDecl(ConvDecl);
+  data().Conversions.addDecl(ConvDecl);
 }
 
 
diff --git a/lib/CodeGen/CGRTTI.cpp b/lib/CodeGen/CGRTTI.cpp
index 29552ce444..5236d20634 100644
--- a/lib/CodeGen/CGRTTI.cpp
+++ b/lib/CodeGen/CGRTTI.cpp
@@ -256,6 +256,9 @@ bool ShouldUseExternalRTTIDescriptor(QualType Ty) {
 
   if (const RecordType *RecordTy = dyn_cast<RecordType>(Ty)) {
     const CXXRecordDecl *RD = cast<CXXRecordDecl>(RecordTy->getDecl());
+    if (!RD->hasDefinition())
+      return false;
+
     if (!RD->isDynamicClass())
       return false;
 
@@ -469,7 +472,7 @@ void RTTIBuilder::BuildVtablePointer(const Type *Ty) {
     const CXXRecordDecl *RD = 
       cast<CXXRecordDecl>(cast<RecordType>(Ty)->getDecl());
     
-    if (!RD->getNumBases()) {
+    if (!RD->hasDefinition() || !RD->getNumBases()) {
       // abi::__class_type_info.
       VtableName = "_ZTVN10__cxxabiv117__class_type_infoE";
     } else if (CanUseSingleInheritance(RD)) {
@@ -566,7 +569,7 @@ llvm::Constant *RTTIBuilder::BuildTypeInfo(QualType Ty) {
   case Type::Record: {
     const CXXRecordDecl *RD = 
       cast<CXXRecordDecl>(cast<RecordType>(Ty)->getDecl());
-    if (!RD->getNumBases()) {
+    if (!RD->hasDefinition() || !RD->getNumBases()) {
       // We don't need to emit any fields.
       break;
     }
diff --git a/lib/CodeGen/CodeGenTypes.cpp b/lib/CodeGen/CodeGenTypes.cpp
index 838f62a5cb..e2f45fe076 100644
--- a/lib/CodeGen/CodeGenTypes.cpp
+++ b/lib/CodeGen/CodeGenTypes.cpp
@@ -399,18 +399,6 @@ const llvm::Type *CodeGenTypes::ConvertNewType(QualType T) {
 /// enum.
 const llvm::Type *CodeGenTypes::ConvertTagDeclType(const TagDecl *TD) {
 
-  // FIXME. This may have to move to a better place.
-  if (const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(TD)) {
-    for (CXXRecordDecl::base_class_const_iterator i = RD->bases_begin(),
-         e = RD->bases_end(); i != e; ++i) {
-      if (!i->isVirtual()) {
-        const CXXRecordDecl *Base =
-          cast<CXXRecordDecl>(i->getType()->getAs<RecordType>()->getDecl());
-        ConvertTagDeclType(Base);
-      }
-    }
-  }
-
   // TagDecl's are not necessarily unique, instead use the (clang)
   // type connected to the decl.
   const Type *Key =
@@ -446,6 +434,18 @@ const llvm::Type *CodeGenTypes::ConvertTagDeclType(const TagDecl *TD) {
 
   const RecordDecl *RD = cast<const RecordDecl>(TD);
 
+  // Force conversion of non-virtual base classes recursively.
+  if (const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(TD)) {    
+    for (CXXRecordDecl::base_class_const_iterator i = RD->bases_begin(),
+         e = RD->bases_end(); i != e; ++i) {
+      if (!i->isVirtual()) {
+        const CXXRecordDecl *Base =
+          cast<CXXRecordDecl>(i->getType()->getAs<RecordType>()->getDecl());
+        ConvertTagDeclType(Base);
+      }
+    }
+  }
+
   // Layout fields.
   CGRecordLayout *Layout = CGRecordLayoutBuilder::ComputeLayout(*this, RD);
 
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 2e3a4352f2..bef53df4f0 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -510,9 +510,18 @@ static void RemoveUsingDecls(LookupResult &R) {
 }
 
 static bool ShouldDiagnoseUnusedDecl(const NamedDecl *D) {
+  if (D->isInvalidDecl())
+    return false;
+
   if (D->isUsed() || D->hasAttr<UnusedAttr>())
     return false;
-  
+
+  // White-list anything that isn't a local variable.
+  if (!isa<VarDecl>(D) || isa<ParmVarDecl>(D) || isa<ImplicitParamDecl>(D) ||
+      !D->getDeclContext()->isFunctionOrMethod())
+    return false;
+
+  // Types of valid local variables should be complete, so this should succeed.
   if (const ValueDecl *VD = dyn_cast<ValueDecl>(D)) {
     if (const RecordType *RT = VD->getType()->getAs<RecordType>()) {
       if (const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(RT->getDecl())) {
@@ -524,9 +533,7 @@ static bool ShouldDiagnoseUnusedDecl(const NamedDecl *D) {
     }
   }
   
-  return (isa<VarDecl>(D) && !isa<ParmVarDecl>(D) && 
-          !isa<ImplicitParamDecl>(D) && 
-          D->getDeclContext()->isFunctionOrMethod());
+  return true;
 }
 
 void Sema::ActOnPopScope(SourceLocation Loc, Scope *S) {
@@ -1903,6 +1910,13 @@ Sema::HandleDeclarator(Scope *S, Declarator &D,
     if (!DC->isDependentContext() && 
         RequireCompleteDeclContext(D.getCXXScopeSpec()))
       return DeclPtrTy();
+
+    if (isa<CXXRecordDecl>(DC) && !cast<CXXRecordDecl>(DC)->hasDefinition()) {
+      Diag(D.getIdentifierLoc(),
+           diag::err_member_def_undefined_record)
+        << Name << DC << D.getCXXScopeSpec().getRange();
+      D.setInvalidType();
+    }
     
     LookupQualifiedName(Previous, DC);
 
@@ -3020,7 +3034,7 @@ Sema::ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC,
 
   if (D.getCXXScopeSpec().isSet() && !NewFD->isInvalidDecl()) {
     // Fake up an access specifier if it's supposed to be a class member.
-    if (isa<CXXRecordDecl>(NewFD->getDeclContext()))
+    if (!Redeclaration && isa<CXXRecordDecl>(NewFD->getDeclContext()))
       NewFD->setAccess(AS_public);
 
     // An out-of-line member function declaration must also be a
@@ -5212,7 +5226,7 @@ FieldDecl *Sema::CheckFieldDecl(DeclarationName Name, QualType T,
     NewFD->setInvalidDecl();
   }
 
-  if (getLangOptions().CPlusPlus) {
+  if (!InvalidDecl && getLangOptions().CPlusPlus) {
     CXXRecordDecl* CXXRecord = cast<CXXRecordDecl>(Record);
 
     if (!T->isPODType())
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index 4552d544b9..e746a8e98c 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -680,7 +680,8 @@ bool Sema::IsDerivedFrom(QualType Derived, QualType Base) {
   
   CXXRecordDecl *DerivedRD = cast<CXXRecordDecl>(DerivedRT->getDecl());
   CXXRecordDecl *BaseRD = cast<CXXRecordDecl>(BaseRT->getDecl());
-  return DerivedRD->isDerivedFrom(BaseRD);
+  // FIXME: instantiate DerivedRD if necessary.  We need a PoI for this.
+  return DerivedRD->hasDefinition() && DerivedRD->isDerivedFrom(BaseRD);
 }
 
 /// \brief Determine whether the type \p Derived is a C++ class that is
@@ -1998,13 +1999,15 @@ bool Sema::RequireNonAbstractType(SourceLocation Loc, QualType T,
   if (!RT)
     return false;
 
-  const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(RT->getDecl());
-  if (!RD)
-    return false;
+  const CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl());
 
   if (CurrentRD && CurrentRD != RD)
     return false;
 
+  // FIXME: is this reasonable?  It matches current behavior, but....
+  if (!RD->getDefinition(Context))
+    return false;
+
   if (!RD->isAbstract())
     return false;
 
diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp
index 850308e147..9033137898 100644
--- a/lib/Sema/SemaExprCXX.cpp
+++ b/lib/Sema/SemaExprCXX.cpp
@@ -72,18 +72,17 @@ Sema::ActOnCXXTypeid(SourceLocation OpLoc, SourceLocation LParenLoc,
       if (const RecordType *RecordT = T->getAs<RecordType>()) {
         CXXRecordDecl *RecordD = cast<CXXRecordDecl>(RecordT->getDecl());
         // C++ [expr.typeid]p3:
+        //   [...] If the type of the expression is a class type, the class
+        //   shall be completely-defined.
+        if (RequireCompleteType(OpLoc, T, diag::err_incomplete_typeid))
+          return ExprError();
+
+        // C++ [expr.typeid]p3:
         //   When typeid is applied to an expression other than an lvalue of a
         //   polymorphic class type [...] [the] expression is an unevaluated
         //   operand. [...]
         if (RecordD->isPolymorphic() && E->isLvalue(Context) == Expr::LV_Valid)
           isUnevaluatedOperand = false;
-        else {
-          // C++ [expr.typeid]p3:
-          //   [...] If the type of the expression is a class type, the class
-          //   shall be completely-defined.
-          if (RequireCompleteType(OpLoc, T, diag::err_incomplete_typeid))
-            return ExprError();
-        }
       }
 
       // C++ [expr.typeid]p4:
@@ -1985,10 +1984,8 @@ Sema::OwningExprResult Sema::MaybeBindToTemporary(Expr *E) {
   if (!RT)
     return Owned(E);
 
-  CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl());
-  if (RD->hasTrivialDestructor())
-    return Owned(E);
-
+  // If this is the result of a call expression, our source might
+  // actually be a reference, in which case we shouldn't bind.
   if (CallExpr *CE = dyn_cast<CallExpr>(E)) {
     QualType Ty = CE->getCallee()->getType();
     if (const PointerType *PT = Ty->getAs<PointerType>())
@@ -1998,6 +1995,13 @@ Sema::OwningExprResult Sema::MaybeBindToTemporary(Expr *E) {
     if (FTy->getResultType()->isReferenceType())
       return Owned(E);
   }
+
+  // That should be enough to guarantee that this type is complete.
+  // If it has a trivial destructor, we can avoid the extra copy.
+  CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl());
+  if (RD->hasTrivialDestructor())
+    return Owned(E);
+
   CXXTemporary *Temp = CXXTemporary::Create(Context,
                                             RD->getDestructor(Context));
   ExprTemporaries.push_back(Temp);
diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp
index d1a379435e..c4b261fad4 100644
--- a/lib/Sema/SemaLookup.cpp
+++ b/lib/Sema/SemaLookup.cpp
@@ -1406,6 +1406,12 @@ addAssociatedClassesAndNamespaces(CXXRecordDecl *Class,
                                         AssociatedClasses);
   }
 
+  // Only recurse into base classes for complete types.
+  if (!Class->hasDefinition()) {
+    // FIXME: we might need to instantiate templates here
+    return;
+  }
+
   // Add direct and indirect base classes along with their associated
   // namespaces.
   llvm::SmallVector<CXXRecordDecl *, 32> Bases;
@@ -2058,6 +2064,9 @@ static void LookupVisibleDecls(DeclContext *Ctx, LookupResult &Result,
 
   // Traverse the contexts of inherited C++ classes.
   if (CXXRecordDecl *Record = dyn_cast<CXXRecordDecl>(Ctx)) {
+    if (!Record->hasDefinition())
+      return;
+
     for (CXXRecordDecl::base_class_iterator B = Record->bases_begin(),
                                          BEnd = Record->bases_end();
          B != BEnd; ++B) {
diff --git a/lib/Sema/SemaOverload.cpp b/lib/Sema/SemaOverload.cpp
index 1319f337a6..c76cbca160 100644
--- a/lib/Sema/SemaOverload.cpp
+++ b/lib/Sema/SemaOverload.cpp
@@ -3408,6 +3408,9 @@ static  Qualifiers CollectVRQualifiers(ASTContext &Context, Expr* ArgExpr) {
     }
     
     CXXRecordDecl *ClassDecl = cast<CXXRecordDecl>(TyRec->getDecl());
+    if (!ClassDecl->hasDefinition())
+      return VRQuals;
+
     const UnresolvedSetImpl *Conversions =
       ClassDecl->getVisibleConversionFunctions();