[cfe-commits] r90752 - in /cfe/trunk: lib/Sema/Sema.cpp lib/Sema/Sema.h lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExpr.cpp test/SemaCXX/virtual-member-functions-key-function.cpp

Anders Carlsson andersca at mac.com
Mon Dec 7 00:25:00 PST 2009


Author: andersca
Date: Mon Dec  7 02:24:59 2009
New Revision: 90752

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

Added:
    cfe/trunk/test/SemaCXX/virtual-member-functions-key-function.cpp
Modified:
    cfe/trunk/lib/Sema/Sema.cpp
    cfe/trunk/lib/Sema/Sema.h
    cfe/trunk/lib/Sema/SemaDecl.cpp
    cfe/trunk/lib/Sema/SemaDeclCXX.cpp
    cfe/trunk/lib/Sema/SemaExpr.cpp

Modified: cfe/trunk/lib/Sema/Sema.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=90752&r1=90751&r2=90752&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/Sema.cpp (original)
+++ cfe/trunk/lib/Sema/Sema.cpp Mon Dec  7 02:24:59 2009
@@ -728,18 +728,27 @@
 /// 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.

Modified: cfe/trunk/lib/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.h?rev=90752&r1=90751&r2=90752&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/Sema.h (original)
+++ cfe/trunk/lib/Sema/Sema.h Mon Dec  7 02:24:59 2009
@@ -32,6 +32,7 @@
 #include "llvm/ADT/OwningPtr.h"
 #include <deque>
 #include <list>
+#include <map>
 #include <string>
 #include <vector>
 
@@ -2147,11 +2148,26 @@
   /// 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);
 

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=90752&r1=90751&r2=90752&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Mon Dec  7 02:24:59 2009
@@ -4114,16 +4114,9 @@
     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");

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=90752&r1=90751&r2=90752&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mon Dec  7 02:24:59 2009
@@ -3163,8 +3163,6 @@
   } else {
     Constructor->setUsed();
   }
-
-  MaybeMarkVirtualImplicitMembersReferenced(CurrentLocation, Constructor);
 }
 
 void Sema::DefineImplicitDestructor(SourceLocation CurrentLocation,
@@ -5083,8 +5081,8 @@
   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 @@
   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 @@
     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);
+  }
+}
+

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=90752&r1=90751&r2=90752&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Dec  7 02:24:59 2009
@@ -6927,6 +6927,8 @@
       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);

Added: cfe/trunk/test/SemaCXX/virtual-member-functions-key-function.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/virtual-member-functions-key-function.cpp?rev=90752&view=auto

==============================================================================
--- cfe/trunk/test/SemaCXX/virtual-member-functions-key-function.cpp (added)
+++ cfe/trunk/test/SemaCXX/virtual-member-functions-key-function.cpp Mon Dec  7 02:24:59 2009
@@ -0,0 +1,22 @@
+// RUN: clang-cc -fsyntax-only -verify %s
+struct A {
+  virtual ~A();
+};
+
+struct B : A {  // expected-error {{no suitable member 'operator delete' in 'B'}}
+  B() { }  // expected-note {{implicit default destructor for 'struct B' first required here}}
+  void operator delete(void *, int); // expected-note {{'operator delete' declared here}}
+};
+
+struct C : A {  // expected-error {{no suitable member 'operator delete' in 'C'}}
+  void operator delete(void *, int); // expected-note {{'operator delete' declared here}}
+};
+
+void f() {
+  // new B should mark the constructor as used, which then marks
+  // all the virtual members as used, because B has no key function.
+  (void)new B;
+
+  // Same here, except that C has an implicit constructor.
+  (void)new C; // expected-note {{implicit default destructor for 'struct C' first required here}}
+}





More information about the cfe-commits mailing list