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.


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

6 files changed, 132 insertions, 23 deletions

diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h
index 3bea485c8f..45854af1bc 100644
--- a/lib/Sema/Sema.h
+++ b/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/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index e32a308af5..dc27dca195 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/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/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp
index 67ad45d74b..e3ca4355e6 100644
--- a/lib/Sema/SemaExprCXX.cpp
+++ b/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;
 }
 
diff --git a/test/CXX/class.access/p4.cpp b/test/CXX/class.access/p4.cpp
index 1cd8966136..e8afbe7a39 100644
--- a/test/CXX/class.access/p4.cpp
+++ b/test/CXX/class.access/p4.cpp
@@ -97,7 +97,7 @@ namespace test2 {
   A A::foo; // okay
   
   class B : A { }; // expected-error {{base class 'test2::A' has private constructor}}
-  B b;
+  B b; // expected-note{{implicit default constructor}}
   
   class C : virtual A { 
   public:
@@ -105,7 +105,7 @@ namespace test2 {
   };
 
   class D : C { }; // expected-error {{inherited virtual base class 'test2::A' has private constructor}}
-  D d;
+  D d; // expected-note{{implicit default constructor}}
 }
 
 // Implicit destructor calls.
@@ -143,13 +143,15 @@ namespace test3 {
   };
 
   class Derived3 : // expected-error 2 {{inherited virtual base class 'Base<2>' has private destructor}} \
-                   // expected-error 2 {{inherited virtual base class 'Base<3>' has private destructor}}
+                   // expected-error 2 {{inherited virtual base class 'Base<3>' has private destructor}} \
+    // expected-note 2{{implicit default constructor}}
     Base<0>,  // expected-error 2 {{base class 'Base<0>' has private destructor}}
     virtual Base<1>, // expected-error 2 {{base class 'Base<1>' has private destructor}}
     Base2, // expected-error 2 {{base class 'test3::Base2' has private destructor}}
     virtual Base3
-  {};
-  Derived3 d3;
+  {}; 
+  Derived3 d3; // expected-note {{implicit default constructor}}\
+               // expected-note{{implicit default destructor}}}
 }
 
 // Conversion functions.
@@ -205,13 +207,13 @@ namespace test5 {
   class Test1 { A a; }; // expected-error {{private member}}
   void test1() {
     Test1 a;
-    a = Test1();
+    a = Test1(); // expected-note{{implicit default copy}}
   }
 
   class Test2 : A {}; // expected-error {{private member}}
   void test2() {
     Test2 a;
-    a = Test2();
+    a = Test2(); // expected-note{{implicit default copy}}
   }
 }
 
@@ -224,12 +226,12 @@ namespace test6 {
 
   class Test1 { A a; }; // expected-error {{field of type 'test6::A' has private copy constructor}}
   void test1(const Test1 &t) {
-    Test1 a = t;
+    Test1 a = t; // expected-note{{implicit default copy}}
   }
 
   class Test2 : A {}; // expected-error {{base class 'test6::A' has private copy constructor}}
   void test2(const Test2 &t) {
-    Test2 a = t;
+    Test2 a = t; // expected-note{{implicit default copy}}
   }
 }
 
diff --git a/test/SemaCXX/default-assignment-operator.cpp b/test/SemaCXX/default-assignment-operator.cpp
index 4b5531e0c8..0be5df39b1 100644
--- a/test/SemaCXX/default-assignment-operator.cpp
+++ b/test/SemaCXX/default-assignment-operator.cpp
@@ -7,7 +7,8 @@ class Base { // expected-error {{cannot define the implicit default assignment o
 };
 
 class X  : Base {  // // expected-error {{cannot define the implicit default assignment operator for 'X', because non-static const member 'cint' can't use default assignment operator}} \
-// expected-note{{assignment operator for 'Base' first required here}}
+// expected-note{{assignment operator for 'Base' first required here}} \
+  // expected-note{{implicit default copy assignment operator}}
 public: 
   X();
   const int cint;  // expected-note {{declared here}}
@@ -28,7 +29,8 @@ Z z2;
 
 // Test1
 void f(X x, const X cx) {
-  x = cx; // expected-note{{assignment operator for 'X' first required here}}
+  x = cx; // expected-note{{assignment operator for 'X' first required here}} \
+  // expected-note{{implicit default copy assignment operator}}
   x = cx;
   z1 = z2;
 }
@@ -84,7 +86,9 @@ public:
 E1 e1, e2;
 
 void j() {
-  e1 = e2; // expected-note{{assignment operator for 'E1' first required here}}
+  // FIXME: duplicated!
+  e1 = e2; // expected-note{{assignment operator for 'E1' first required here}} \
+  // expected-note{{implicit default copy assignment operator}}
 }
 
 namespace ProtectedCheck {
@@ -101,7 +105,8 @@ namespace ProtectedCheck {
     X x;
   };
 
-  void f(Z z) { z = z; } // 
+  void f(Z z) { z = z; }  // expected-note{{implicit default copy assignment operator}}
+
 }
 
 namespace MultiplePaths {
diff --git a/test/SemaTemplate/virtual-member-functions.cpp b/test/SemaTemplate/virtual-member-functions.cpp
index 59df3c22aa..9ef1849bce 100644
--- a/test/SemaTemplate/virtual-member-functions.cpp
+++ b/test/SemaTemplate/virtual-member-functions.cpp
@@ -53,3 +53,26 @@ T *HasOutOfLineKey<T>::f(float *fp) {
 }
 
 HasOutOfLineKey<int> out_of_line;
+
+namespace std {
+  class type_info;
+}
+
+namespace PR7114 {
+  class A { virtual ~A(); }; // expected-note{{declared private here}}
+
+  template<typename T>
+  class B {
+  public:
+    class Inner : public A { }; // expected-error{{base class 'PR7114::A' has private destructor}}
+    static Inner i;
+    static const unsigned value = sizeof(i) == 4;
+  };
+
+  int f() { return B<int>::value; }
+
+  void test_typeid(B<float>::Inner bfi) {
+    (void)typeid(bfi); // expected-note{{implicit default destructor}}
+  }
+}
+