[cfe-commits] r103497 - in /cfe/trunk: lib/Sema/Sema.h lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExprCXX.cpp test/CXX/class.access/p4.cpp test/SemaCXX/default-assignment-operator.cpp test/SemaTemplate/virtual-member-functions.cpp

Douglas Gregor dgregor at apple.com
Tue May 11 13:24:17 PDT 2010


Author: dgregor
Date: Tue May 11 15:24:17 2010
New Revision: 103497

URL: http://llvm.org/viewvc/llvm-project?rev=103497&view=rev
Log:
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.

Modified:
    cfe/trunk/lib/Sema/Sema.h
    cfe/trunk/lib/Sema/SemaDeclCXX.cpp
    cfe/trunk/lib/Sema/SemaExprCXX.cpp
    cfe/trunk/test/CXX/class.access/p4.cpp
    cfe/trunk/test/SemaCXX/default-assignment-operator.cpp
    cfe/trunk/test/SemaTemplate/virtual-member-functions.cpp

Modified: cfe/trunk/lib/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.h?rev=103497&r1=103496&r2=103497&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/Sema.h (original)
+++ cfe/trunk/lib/Sema/Sema.h Tue May 11 15:24:17 2010
@@ -2563,11 +2563,19 @@
   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 @@
     }
   };
 
+  /// \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.
   ///

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=103497&r1=103496&r2=103497&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Tue May 11 15:24:17 2010
@@ -4146,7 +4146,9 @@
   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 @@
   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 @@
   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 @@
     }
   }
 
+  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 @@
   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 @@
     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 @@
     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);
   }
   

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=103497&r1=103496&r2=103497&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Tue May 11 15:24:17 2010
@@ -287,7 +287,7 @@
   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 @@
       //   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 @@
   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;
 }
 

Modified: cfe/trunk/test/CXX/class.access/p4.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/class.access/p4.cpp?rev=103497&r1=103496&r2=103497&view=diff
==============================================================================
--- cfe/trunk/test/CXX/class.access/p4.cpp (original)
+++ cfe/trunk/test/CXX/class.access/p4.cpp Tue May 11 15:24:17 2010
@@ -97,7 +97,7 @@
   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 @@
   };
 
   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 @@
   };
 
   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 @@
   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 @@
 
   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}}
   }
 }
 

Modified: cfe/trunk/test/SemaCXX/default-assignment-operator.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/default-assignment-operator.cpp?rev=103497&r1=103496&r2=103497&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/default-assignment-operator.cpp (original)
+++ cfe/trunk/test/SemaCXX/default-assignment-operator.cpp Tue May 11 15:24:17 2010
@@ -7,7 +7,8 @@
 };
 
 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 @@
 
 // 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 @@
 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 @@
     X x;
   };
 
-  void f(Z z) { z = z; } // 
+  void f(Z z) { z = z; }  // expected-note{{implicit default copy assignment operator}}
+
 }
 
 namespace MultiplePaths {

Modified: cfe/trunk/test/SemaTemplate/virtual-member-functions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/virtual-member-functions.cpp?rev=103497&r1=103496&r2=103497&view=diff
==============================================================================
--- cfe/trunk/test/SemaTemplate/virtual-member-functions.cpp (original)
+++ cfe/trunk/test/SemaTemplate/virtual-member-functions.cpp Tue May 11 15:24:17 2010
@@ -53,3 +53,26 @@
 }
 
 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}}
+  }
+}
+





More information about the cfe-commits mailing list