Rework how virtual member functions are marked. If a class has no key function, we now wait until the end of the translation unit to mark its virtual member functions as references. This lays the groundwork for fixing PR5557.

llvm-svn: 90752

5 files changed, 96 insertions, 33 deletions

diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 29efbd7becd..ef6147420be 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -728,18 +728,27 @@ void Sema::DeleteStmt(StmtTy *S) {
 /// translation unit when EOF is reached and all but the top-level scope is
 /// popped.
 void Sema::ActOnEndOfTranslationUnit() {
-  // C++: Perform implicit template instantiations.
-  //
-  // FIXME: When we perform these implicit instantiations, we do not carefully
-  // keep track of the point of instantiation (C++ [temp.point]). This means
-  // that name lookup that occurs within the template instantiation will
-  // always happen at the end of the translation unit, so it will find
-  // some names that should not be found. Although this is common behavior
-  // for C++ compilers, it is technically wrong. In the future, we either need
-  // to be able to filter the results of name lookup or we need to perform
-  // template instantiations earlier.
-  PerformPendingImplicitInstantiations();
-
+  
+  while (1) {
+    // C++: Perform implicit template instantiations.
+    //
+    // FIXME: When we perform these implicit instantiations, we do not carefully
+    // keep track of the point of instantiation (C++ [temp.point]). This means
+    // that name lookup that occurs within the template instantiation will
+    // always happen at the end of the translation unit, so it will find
+    // some names that should not be found. Although this is common behavior
+    // for C++ compilers, it is technically wrong. In the future, we either need
+    // to be able to filter the results of name lookup or we need to perform
+    // template instantiations earlier.
+    PerformPendingImplicitInstantiations();
+    
+    /// If ProcessPendingClassesWithUnmarkedVirtualMembers ends up marking 
+    /// any virtual member functions it might lead to more pending template
+    /// instantiations, which is why we need to loop here.
+    if (!ProcessPendingClassesWithUnmarkedVirtualMembers())
+      break;
+  }
+  
   // Check for #pragma weak identifiers that were never declared
   // FIXME: This will cause diagnostics to be emitted in a non-determinstic
   // order!  Iterating over a densemap like this is bad.
diff --git a/clang/lib/Sema/Sema.h b/clang/lib/Sema/Sema.h
index e4ee8ce73cc..edb4528f7e6 100644
--- a/clang/lib/Sema/Sema.h
+++ b/clang/lib/Sema/Sema.h
@@ -32,6 +32,7 @@
 #include "llvm/ADT/OwningPtr.h"
 #include <deque>
 #include <list>
+#include <map>
 #include <string>
 #include <vector>
 
@@ -2147,11 +2148,26 @@ public:
   /// as referenced.
   void MarkBaseAndMemberDestructorsReferenced(CXXDestructorDecl *Destructor);
 
-  /// MaybeMarkVirtualImplicitMembersReferenced - If the passed in method is the
+  /// ClassesWithUnmarkedVirtualMembers - Contains record decls whose virtual
+  /// members might need to be marked as referenced. This is either done when
+  /// the key function definition is emitted (this is handled by by 
+  /// MaybeMarkVirtualMembersReferenced), or at the end of the translation unit
+  /// (done by ProcessPendingClassesWithUnmarkedVirtualMembers).
+  std::map<CXXRecordDecl *, SourceLocation> ClassesWithUnmarkedVirtualMembers;
+
+  /// MaybeMarkVirtualMembersReferenced - If the passed in method is the
   /// key function of the record decl, will mark virtual member functions as 
   /// referenced.
-  void MaybeMarkVirtualImplicitMembersReferenced(SourceLocation Loc, 
-                                                 CXXMethodDecl *MD);
+  void MaybeMarkVirtualMembersReferenced(SourceLocation Loc, CXXMethodDecl *MD);
+  
+  /// MarkVirtualMembersReferenced - Will mark all virtual members of the given
+  /// CXXRecordDecl referenced.
+  void MarkVirtualMembersReferenced(SourceLocation Loc, CXXRecordDecl *RD);
+
+  /// ProcessPendingClassesWithUnmarkedVirtualMembers - Will process classes 
+  /// that might need to have their virtual members marked as referenced.
+  /// Returns false if no work was done.
+  bool ProcessPendingClassesWithUnmarkedVirtualMembers();
   
   void AddImplicitlyDeclaredMembersToClass(CXXRecordDecl *ClassDecl);
 
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index a5314bb075e..bb3f8694250 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -4114,16 +4114,9 @@ Sema::DeclPtrTy Sema::ActOnFinishFunctionBody(DeclPtrTy D, StmtArg BodyArg,
     if (!FD->isInvalidDecl())
       DiagnoseUnusedParameters(FD->param_begin(), FD->param_end());
 
-    if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(FD)) {
-      // C++ [basic.def.odr]p2:
-      //   [...] A virtual member function is used if it is not pure. [...]
-      if (Method->isVirtual() && !Method->isPure())
-        MarkDeclarationReferenced(Method->getLocation(), Method);
-
-      if (!Method->isInlined())
-        MaybeMarkVirtualImplicitMembersReferenced(Method->getLocation(), 
-                                                  Method);
-    }
+    if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(FD))
+      MaybeMarkVirtualMembersReferenced(Method->getLocation(), Method);
+
     assert(FD == getCurFunctionDecl() && "Function parsing confused");
   } else if (ObjCMethodDecl *MD = dyn_cast_or_null<ObjCMethodDecl>(dcl)) {
     assert(MD == getCurMethodDecl() && "Method parsing confused");
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 1e96bf525ae..bd191ab8c96 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -3163,8 +3163,6 @@ void Sema::DefineImplicitDefaultConstructor(SourceLocation CurrentLocation,
   } else {
     Constructor->setUsed();
   }
-
-  MaybeMarkVirtualImplicitMembersReferenced(CurrentLocation, Constructor);
 }
 
 void Sema::DefineImplicitDestructor(SourceLocation CurrentLocation,
@@ -5083,8 +5081,8 @@ Sema::ActOnCXXConditionDeclaration(Scope *S, Declarator &D) {
   return Dcl;
 }
 
-void Sema::MaybeMarkVirtualImplicitMembersReferenced(SourceLocation Loc,
-                                                     CXXMethodDecl *MD) {
+void Sema::MaybeMarkVirtualMembersReferenced(SourceLocation Loc,
+                                             CXXMethodDecl *MD) {
   // Ignore dependent types.
   if (MD->isDependentContext())
     return;
@@ -5095,6 +5093,16 @@ void Sema::MaybeMarkVirtualImplicitMembersReferenced(SourceLocation Loc,
   if (!RD->isDynamicClass())
     return;
 
+  if (!MD->isOutOfLine()) {
+    // The only inline functions we care about are constructors. We also defer
+    // marking the virtual members as referenced until we've reached the end
+    // of the translation unit. We do this because we need to know the key
+    // function of the class in order to determine the key function.
+    if (isa<CXXConstructorDecl>(MD))
+      ClassesWithUnmarkedVirtualMembers.insert(std::make_pair(RD, Loc));
+    return;
+  }
+  
   const CXXMethodDecl *KeyFunction = Context.getKeyFunction(RD);
 
   if (!KeyFunction) {
@@ -5107,10 +5115,45 @@ void Sema::MaybeMarkVirtualImplicitMembersReferenced(SourceLocation Loc,
     return;
   }
   
-  if (CXXDestructorDecl *Dtor = RD->getDestructor(Context)) {
-    if (Dtor->isImplicit() && Dtor->isVirtual())
-      MarkDeclarationReferenced(Loc, Dtor);
+  // Mark the members as referenced.
+  MarkVirtualMembersReferenced(Loc, RD);
+  ClassesWithUnmarkedVirtualMembers.erase(RD);
+}
+
+bool Sema::ProcessPendingClassesWithUnmarkedVirtualMembers() {
+  if (ClassesWithUnmarkedVirtualMembers.empty())
+    return false;
+  
+  for (std::map<CXXRecordDecl *, SourceLocation>::iterator i = 
+       ClassesWithUnmarkedVirtualMembers.begin(), 
+       e = ClassesWithUnmarkedVirtualMembers.end(); i != e; ++i) {
+    CXXRecordDecl *RD = i->first;
+    
+    const CXXMethodDecl *KeyFunction = Context.getKeyFunction(RD);
+    if (KeyFunction) {
+      // We know that the class has a key function. If the key function was
+      // declared in this translation unit, then it the class decl would not 
+      // have been in the ClassesWithUnmarkedVirtualMembers map.
+      continue;
+    }
+    
+    SourceLocation Loc = i->second;
+    MarkVirtualMembersReferenced(Loc, RD);
   }
   
-  // FIXME: Need to handle the virtual assignment operator here too.
+  ClassesWithUnmarkedVirtualMembers.clear();
+  return true;
 }
+
+void Sema::MarkVirtualMembersReferenced(SourceLocation Loc, CXXRecordDecl *RD) {
+  for (CXXRecordDecl::method_iterator i = RD->method_begin(), 
+       e = RD->method_end(); i != e; ++i) {
+    CXXMethodDecl *MD = *i;
+
+    // C++ [basic.def.odr]p2:
+    //   [...] A virtual member function is used if it is not pure. [...]
+    if (MD->isVirtual() && !MD->isPure())
+      MarkDeclarationReferenced(Loc, MD);
+  }
+}
+
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 987de9b5b45..6c5a1ec05c5 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -6927,6 +6927,8 @@ void Sema::MarkDeclarationReferenced(SourceLocation Loc, Decl *D) {
       if (!Constructor->isUsed())
         DefineImplicitCopyConstructor(Loc, Constructor, TypeQuals);
     }
+    
+    MaybeMarkVirtualMembersReferenced(Loc, Constructor);
   } else if (CXXDestructorDecl *Destructor = dyn_cast<CXXDestructorDecl>(D)) {
     if (Destructor->isImplicit() && !Destructor->isUsed())
       DefineImplicitDestructor(Loc, Destructor);