Do not mark the virtual members of an implicitly-instantiated class as

referenced unless we see one of them defined (or the key function
defined, if it as one) or if we need the vtable for something. Fixes
PR7114.

llvm-svn: 103497

3 files changed, 89 insertions, 10 deletions

diff --git a/clang/lib/Sema/Sema.h b/clang/lib/Sema/Sema.h
index 3bea485c8fb..45854af1bc5 100644
--- a/clang/lib/Sema/Sema.h
+++ b/clang/lib/Sema/Sema.h
@@ -2563,11 +2563,19 @@ public:
   llvm::SmallVector<std::pair<CXXRecordDecl *, SourceLocation>, 4>
     ClassesWithUnmarkedVirtualMembers;
 
+  /// \brief Contains the set of classes with unmarked virtual members
+  /// that require a vtable.
+  llvm::SmallPtrSet<CXXRecordDecl *, 4> UnmarkedClassesRequiringVtable;
+
   /// MaybeMarkVirtualMembersReferenced - If the passed in method is the
   /// key function of the record decl, will mark virtual member functions as
   /// referenced.
   void MaybeMarkVirtualMembersReferenced(SourceLocation Loc, CXXMethodDecl *MD);
 
+  /// \brief If the given class does not have a key function, mark its
+  /// virtual members as referenced.
+  void MaybeMarkVirtualMembersReferenced(SourceLocation Loc, CXXRecordDecl *RD);
+
   /// MarkVirtualMembersReferenced - Will mark all virtual members of the given
   /// CXXRecordDecl referenced.
   void MarkVirtualMembersReferenced(SourceLocation Loc,
@@ -3571,6 +3579,24 @@ public:
     }
   };
 
+  /// \brief RAII class that determines when any errors have occurred
+  /// between the time the instance was created and the time it was
+  /// queried.
+  class ErrorTrap {
+    Sema &SemaRef;
+    unsigned PrevErrors;
+
+  public:
+    explicit ErrorTrap(Sema &SemaRef)
+      : SemaRef(SemaRef), PrevErrors(SemaRef.getDiagnostics().getNumErrors()) {}
+
+    /// \brief Determine whether any errors have occurred since this
+    /// object instance was created.
+    bool hasErrorOccurred() const {
+      return SemaRef.getDiagnostics().getNumErrors() > PrevErrors;
+    }
+  };
+
   /// \brief A stack-allocated class that identifies which local
   /// variable declaration instantiations are present in this scope.
   ///
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index e32a308af58..dc27dca1958 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -4146,7 +4146,9 @@ void Sema::DefineImplicitDefaultConstructor(SourceLocation CurrentLocation,
   assert(ClassDecl && "DefineImplicitDefaultConstructor - invalid constructor");
 
   ImplicitlyDefinedFunctionScope Scope(*this, Constructor);
-  if (SetBaseOrMemberInitializers(Constructor, 0, 0, /*AnyErrors=*/false)) {
+  ErrorTrap Trap(*this);
+  if (SetBaseOrMemberInitializers(Constructor, 0, 0, /*AnyErrors=*/false) ||
+      Trap.hasErrorOccurred()) {
     Diag(CurrentLocation, diag::note_member_synthesized_at) 
       << CXXConstructor << Context.getTagDeclType(ClassDecl);
     Constructor->setInvalidDecl();
@@ -4162,14 +4164,16 @@ void Sema::DefineImplicitDestructor(SourceLocation CurrentLocation,
   CXXRecordDecl *ClassDecl = Destructor->getParent();
   assert(ClassDecl && "DefineImplicitDestructor - invalid destructor");
 
+  if (Destructor->isInvalidDecl())
+    return;
+
   ImplicitlyDefinedFunctionScope Scope(*this, Destructor);
 
+  ErrorTrap Trap(*this);
   MarkBaseAndMemberDestructorsReferenced(Destructor->getLocation(),
                                          Destructor->getParent());
 
-  // FIXME: If CheckDestructor fails, we should emit a note about where the
-  // implicit destructor was needed.
-  if (CheckDestructor(Destructor)) {
+  if (CheckDestructor(Destructor) || Trap.hasErrorOccurred()) {
     Diag(CurrentLocation, diag::note_member_synthesized_at) 
       << CXXDestructor << Context.getTagDeclType(ClassDecl);
 
@@ -4396,6 +4400,7 @@ void Sema::DefineImplicitCopyAssignment(SourceLocation CurrentLocation,
   CopyAssignOperator->setUsed();
 
   ImplicitlyDefinedFunctionScope Scope(*this, CopyAssignOperator);
+  ErrorTrap Trap(*this);
 
   // C++0x [class.copy]p30:
   //   The implicitly-defined or explicitly-defaulted copy assignment operator
@@ -4619,6 +4624,13 @@ void Sema::DefineImplicitCopyAssignment(SourceLocation CurrentLocation,
     }
   }
 
+  if (Trap.hasErrorOccurred()) {
+    Diag(CurrentLocation, diag::note_member_synthesized_at) 
+      << CXXCopyAssignment << Context.getTagDeclType(ClassDecl);
+    CopyAssignOperator->setInvalidDecl();
+    return;
+  }
+
   if (Invalid) {
     CopyAssignOperator->setInvalidDecl();
     return;
@@ -4642,8 +4654,10 @@ void Sema::DefineImplicitCopyConstructor(SourceLocation CurrentLocation,
   assert(ClassDecl && "DefineImplicitCopyConstructor - invalid constructor");
 
   ImplicitlyDefinedFunctionScope Scope(*this, CopyConstructor);
+  ErrorTrap Trap(*this);
 
-  if (SetBaseOrMemberInitializers(CopyConstructor, 0, 0, /*AnyErrors=*/false)) {
+  if (SetBaseOrMemberInitializers(CopyConstructor, 0, 0, /*AnyErrors=*/false) ||
+      Trap.hasErrorOccurred()) {
     Diag(CurrentLocation, diag::note_member_synthesized_at) 
       << CXXCopyConstructor << Context.getTagDeclType(ClassDecl);
     CopyConstructor->setInvalidDecl();
@@ -6068,12 +6082,32 @@ void Sema::MaybeMarkVirtualMembersReferenced(SourceLocation Loc,
     return;
 
   TemplateSpecializationKind kind = RD->getTemplateSpecializationKind();
-  if (kind == TSK_ImplicitInstantiation)
-    ClassesWithUnmarkedVirtualMembers.push_back(std::make_pair(RD, Loc));
-  else
+  if (kind == TSK_ImplicitInstantiation) {
+    CXXRecordDecl *CanonRD = cast<CXXRecordDecl>(RD->getCanonicalDecl());
+    if (UnmarkedClassesRequiringVtable.insert(CanonRD))
+      ClassesWithUnmarkedVirtualMembers.push_back(std::make_pair(RD, Loc));
+  } else
     MarkVirtualMembersReferenced(Loc, RD);
 }
 
+void Sema::MaybeMarkVirtualMembersReferenced(SourceLocation Loc, 
+                                             CXXRecordDecl *RD) {
+  if (RD->isDependentContext() || !RD->isDynamicClass())
+    return;
+
+  if (RD->getTemplateSpecializationKind() != TSK_ImplicitInstantiation)
+    return;
+
+  // Classes with a key function will be dealt with when we see the
+  // definition of the key function.
+  if (Context.getKeyFunction(RD))
+    return;
+
+  CXXRecordDecl *CanonRD = cast<CXXRecordDecl>(RD->getCanonicalDecl());
+  if (UnmarkedClassesRequiringVtable.insert(CanonRD))
+    ClassesWithUnmarkedVirtualMembers.push_back(std::make_pair(RD, Loc));
+}
+
 bool Sema::ProcessPendingClassesWithUnmarkedVirtualMembers() {
   if (ClassesWithUnmarkedVirtualMembers.empty())
     return false;
@@ -6082,6 +6116,15 @@ bool Sema::ProcessPendingClassesWithUnmarkedVirtualMembers() {
     CXXRecordDecl *RD = ClassesWithUnmarkedVirtualMembers.back().first;
     SourceLocation Loc = ClassesWithUnmarkedVirtualMembers.back().second;
     ClassesWithUnmarkedVirtualMembers.pop_back();
+
+    // If an implicitly-instantiated class doesn't require a vtable,
+    // don't mark its virtual members as referenced.
+    if (RD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation) {
+      CXXRecordDecl *CanonRD = cast<CXXRecordDecl>(RD->getCanonicalDecl());
+      if (!UnmarkedClassesRequiringVtable.count(CanonRD))
+        continue;
+    }
+
     MarkVirtualMembersReferenced(Loc, RD);
   }
   
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 67ad45d74b2..e3ca4355e65 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -287,7 +287,7 @@ Sema::OwningExprResult Sema::BuildCXXTypeId(QualType TypeInfoType,
   if (T->getAs<RecordType>() &&
       RequireCompleteType(TypeidLoc, T, diag::err_incomplete_typeid))
     return ExprError();
-  
+
   return Owned(new (Context) CXXTypeidExpr(TypeInfoType.withConst(),
                                            Operand,
                                            SourceRange(TypeidLoc, RParenLoc)));
@@ -314,8 +314,10 @@ Sema::OwningExprResult Sema::BuildCXXTypeId(QualType TypeInfoType,
       //   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)
+      if (RecordD->isPolymorphic() && E->isLvalue(Context) == Expr::LV_Valid) {
         isUnevaluatedOperand = false;
+        MaybeMarkVirtualMembersReferenced(TypeidLoc, RecordD);
+      }
     }
     
     // C++ [expr.typeid]p4:
@@ -445,6 +447,14 @@ bool Sema::CheckCXXThrowOperand(SourceLocation ThrowLoc, Expr *&E) {
   if (Res.isInvalid())
     return true;
   E = Res.takeAs<Expr>();
+
+  if (const RecordType *RecordTy = Ty->getAs<RecordType>()) {
+    // If we're throwing a polymorphic class, we need to make sure
+    // there is a vtable.
+    MaybeMarkVirtualMembersReferenced(ThrowLoc, 
+                                      cast<CXXRecordDecl>(RecordTy->getDecl()));
+  }
+
   return false;
 }