Fix a bug where we would add the same function twice in a vtable.

llvm-svn: 100949

2 files changed, 58 insertions, 18 deletions

diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp
index a11a2d61975..93b555fd773 100644
--- a/clang/lib/CodeGen/CGVTables.cpp
+++ b/clang/lib/CodeGen/CGVTables.cpp
@@ -1361,24 +1361,23 @@ void VTableBuilder::AddThunk(const CXXMethodDecl *MD, const ThunkInfo &Thunk) {
   ThunksVector.push_back(Thunk);
 }
 
-/// OverridesMethodInBases - Checks whether whether this virtual member 
-/// function overrides a member function in any of the given bases.
-/// Returns the overridden member function, or null if none was found.
-static const CXXMethodDecl * 
-OverridesMethodInBases(const CXXMethodDecl *MD,
-                       VTableBuilder::PrimaryBasesSetVectorTy &Bases) {
+typedef llvm::SmallPtrSet<const CXXMethodDecl *, 8> OverriddenMethodsSetTy;
+
+/// ComputeAllOverriddenMethods - Given a method decl, will return a set of all
+/// the overridden methods that the function decl overrides.
+static void 
+ComputeAllOverriddenMethods(const CXXMethodDecl *MD,
+                            OverriddenMethodsSetTy& OverriddenMethods) {
+  assert(MD->isVirtual() && "Method is not virtual!");
+
   for (CXXMethodDecl::method_iterator I = MD->begin_overridden_methods(),
        E = MD->end_overridden_methods(); I != E; ++I) {
     const CXXMethodDecl *OverriddenMD = *I;
-    const CXXRecordDecl *OverriddenRD = OverriddenMD->getParent();
-    assert(OverriddenMD->isCanonicalDecl() &&
-           "Should have the canonical decl of the overridden RD!");
     
-    if (Bases.count(OverriddenRD))
-      return OverriddenMD;
+    OverriddenMethods.insert(OverriddenMD);
+    
+    ComputeAllOverriddenMethods(OverriddenMD, OverriddenMethods);
   }
-      
-  return 0;
 }
 
 void VTableBuilder::ComputeThisAdjustments() {
@@ -1616,7 +1615,7 @@ VTableBuilder::AddMethod(const CXXMethodDecl *MD,
 /// struct C : B { virtual void f(); }
 ///
 /// OverridesIndirectMethodInBase will return true if given C::f as the method 
-/// and { A } as the set of  bases.
+/// and { A } as the set of bases.
 static bool
 OverridesIndirectMethodInBases(const CXXMethodDecl *MD,
                                VTableBuilder::PrimaryBasesSetVectorTy &Bases) {
@@ -1706,14 +1705,17 @@ VTableBuilder::IsOverriderUsed(const CXXMethodDecl *Overrider,
 static const CXXMethodDecl * 
 FindNearestOverriddenMethod(const CXXMethodDecl *MD,
                             VTableBuilder::PrimaryBasesSetVectorTy &Bases) {
+  OverriddenMethodsSetTy OverriddenMethods;
+  ComputeAllOverriddenMethods(MD, OverriddenMethods);
+  
   for (int I = Bases.size(), E = 0; I != E; --I) {
     const CXXRecordDecl *PrimaryBase = Bases[I - 1];
 
     // Now check the overriden methods.
-    for (CXXMethodDecl::method_iterator I = MD->begin_overridden_methods(),
-         E = MD->end_overridden_methods(); I != E; ++I) {
+    for (OverriddenMethodsSetTy::const_iterator I = OverriddenMethods.begin(),
+         E = OverriddenMethods.end(); I != E; ++I) {
       const CXXMethodDecl *OverriddenMD = *I;
-
+      
       // We found our overridden method.
       if (OverriddenMD->getParent() == PrimaryBase)
         return OverriddenMD;
@@ -2418,7 +2420,7 @@ void CodeGenVTables::ComputeMethodVtableIndices(const CXXRecordDecl *RD) {
 
     // Check if this method overrides a method in the primary base.
     if (const CXXMethodDecl *OverriddenMD = 
-          OverridesMethodInBases(MD, PrimaryBases)) {
+          FindNearestOverriddenMethod(MD, PrimaryBases)) {
       // Check if converting from the return type of the method to the 
       // return type of the overridden method requires conversion.
       if (ComputeReturnAdjustmentBaseOffset(CGM.getContext(), MD, 
diff --git a/clang/test/CodeGenCXX/vtable-layout.cpp b/clang/test/CodeGenCXX/vtable-layout.cpp
index 1d34b240f32..62e02fcea5e 100644
--- a/clang/test/CodeGenCXX/vtable-layout.cpp
+++ b/clang/test/CodeGenCXX/vtable-layout.cpp
@@ -1326,3 +1326,41 @@ struct F : E {
 void F::f() { }
 
 }
+
+namespace Test31 {
+
+// Test that we don't add D::f twice to the primary vtable.
+struct A {
+  int a;
+};
+
+struct B {
+  virtual void f();
+};
+
+struct C : A, virtual B {
+  virtual void f();
+};
+
+// CHECK:      Vtable for 'Test31::D' (11 entries).
+// CHECK-NEXT:    0 | vbase_offset (0)
+// CHECK-NEXT:    1 | vbase_offset (8)
+// CHECK-NEXT:    2 | vcall_offset (0)
+// CHECK-NEXT:    3 | offset_to_top (0)
+// CHECK-NEXT:    4 | Test31::D RTTI
+// CHECK-NEXT:        -- (Test31::B, 0) vtable address --
+// CHECK-NEXT:        -- (Test31::D, 0) vtable address --
+// CHECK-NEXT:    5 | void Test31::D::f()
+// CHECK-NEXT:    6 | vbase_offset (-8)
+// CHECK-NEXT:    7 | vcall_offset (-8)
+// CHECK-NEXT:    8 | offset_to_top (-8)
+// CHECK-NEXT:    9 | Test31::D RTTI
+// CHECK-NEXT:        -- (Test31::C, 8) vtable address --
+// CHECK-NEXT:   10 | void Test31::D::f()
+// CHECK-NEXT:        [this adjustment: 0 non-virtual, -24 vcall offset offset]
+struct D : virtual C {
+  virtual void f();
+};
+void D::f() { }
+
+}