r193610 - Fix an assertion when handling a custom case of virtual inheritance; also reduce code duplication

[Timur Iskhodzhanov]

Author: timurrrr
Date: Tue Oct 29 09:13:45 2013
New Revision: 193610

URL: http://llvm.org/viewvc/llvm-project?rev=193610&view=rev
Log:
Fix an assertion when handling a custom case of virtual inheritance; also reduce code duplication

Modified:
    cfe/trunk/lib/AST/VTableBuilder.cpp
    cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp

Modified: cfe/trunk/lib/AST/VTableBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/VTableBuilder.cpp?rev=193610&r1=193609&r2=193610&view=diff
==============================================================================
--- cfe/trunk/lib/AST/VTableBuilder.cpp (original)
+++ cfe/trunk/lib/AST/VTableBuilder.cpp Tue Oct 29 09:13:45 2013
@@ -2730,28 +2730,6 @@ VFTableBuilder::ComputeThisOffset(const
   return Ret;
 }
 
-static const CXXMethodDecl*
-FindDirectlyOverriddenMethodInBases(const CXXMethodDecl *MD,
-                                    BasesSetVectorTy &Bases) {
-  // We can't just iterate over the overridden methods and return the first one
-  // which has its parent in Bases, e.g. this doesn't work when we have
-  // multiple subobjects of the same type that have its virtual function
-  // overridden.
-  for (int I = Bases.size(), E = 0; I != E; --I) {
-    const CXXRecordDecl *CurrentBase = Bases[I - 1];
-
-    for (CXXMethodDecl::method_iterator I = MD->begin_overridden_methods(),
-         E = MD->end_overridden_methods(); I != E; ++I) {
-      const CXXMethodDecl *OverriddenMD = *I;
-
-      if (OverriddenMD->getParent() == CurrentBase)
-        return OverriddenMD;
-    }
-  }
-
-  return 0;
-}
-
 static void GroupNewVirtualOverloads(
     const CXXRecordDecl *RD,
     SmallVector<const CXXMethodDecl *, 10> &VirtualMethods) {
@@ -2843,7 +2821,7 @@ void VFTableBuilder::AddMethods(BaseSubo
     // Check if this virtual member function overrides
     // a method in one of the visited bases.
     if (const CXXMethodDecl *OverriddenMD =
-            FindDirectlyOverriddenMethodInBases(MD, VisitedBases)) {
+            FindNearestOverriddenMethod(MD, VisitedBases)) {
       MethodInfoMapTy::iterator OverriddenMDIterator =
           MethodInfoMap.find(OverriddenMD);
 
@@ -2887,7 +2865,7 @@ void VFTableBuilder::AddMethods(BaseSubo
           // FIXME: this is O(N^2), can be O(N).
           const CXXMethodDecl *SubOverride = OverriddenMD;
           while ((SubOverride =
-              FindDirectlyOverriddenMethodInBases(SubOverride, VisitedBases))) {
+                      FindNearestOverriddenMethod(SubOverride, VisitedBases))) {
             MethodInfoMapTy::iterator SubOverrideIterator =
                 MethodInfoMap.find(SubOverride);
             if (SubOverrideIterator == MethodInfoMap.end())

Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp?rev=193610&r1=193609&r2=193610&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp Tue Oct 29 09:13:45 2013
@@ -9,7 +9,8 @@
 // RUN: FileCheck --check-prefix=TEST5 %s < %t
 // RUN: FileCheck --check-prefix=TEST6 %s < %t
 // RUN: FileCheck --check-prefix=TEST7 %s < %t
-// RUN: FileCheck --check-prefix=TEST8 %s < %t
+// RUN: FileCheck --check-prefix=TEST8-X %s < %t
+// RUN: FileCheck --check-prefix=TEST8-Z %s < %t
 // RUN: FileCheck --check-prefix=TEST9-Y %s < %t
 // RUN: FileCheck --check-prefix=TEST9-Z %s < %t
 // RUN: FileCheck --check-prefix=TEST9-W %s < %t
@@ -236,16 +237,16 @@ namespace Test8 {
 
 // This is a typical diamond inheritance with a shared 'A' vbase.
 struct X : D, C {
-  // TEST8: VFTable for 'D' in 'Test8::X' (1 entries).
-  // TEST8-NEXT: 0 | void D::h()
+  // TEST8-X: VFTable for 'D' in 'Test8::X' (1 entries).
+  // TEST8-X-NEXT: 0 | void D::h()
 
-  // TEST8: VFTable for 'A' in 'D' in 'Test8::X' (2 entries).
-  // TEST8-NEXT: 0 | void Test8::X::f()
-  // TEST8-NEXT: 1 | void A::z()
-
-  // TEST8: VFTable indices for 'Test8::X' (1 entries).
-  // TEST8-NEXT: via vbtable index 1, vfptr at offset 0
-  // TEST8-NEXT: 0 | void Test8::X::f()
+  // TEST8-X: VFTable for 'A' in 'D' in 'Test8::X' (2 entries).
+  // TEST8-X-NEXT: 0 | void Test8::X::f()
+  // TEST8-X-NEXT: 1 | void A::z()
+
+  // TEST8-X: VFTable indices for 'Test8::X' (1 entries).
+  // TEST8-X-NEXT: via vbtable index 1, vfptr at offset 0
+  // TEST8-X-NEXT: 0 | void Test8::X::f()
 
   // MANGLING-DAG: @"\01??_7X at Test8@@6BA@@@"
   // MANGLING-DAG: @"\01??_7X at Test8@@6BD@@@"
@@ -254,6 +255,21 @@ struct X : D, C {
 };
 
 X x;
+
+// Another diamond inheritance which led to AST crashes.
+struct Y : virtual A {};
+
+class Z : Y, C {
+  // TEST8-Z: VFTable for 'A' in 'Test8::Y' in 'Test8::Z' (2 entries).
+  // TEST8-Z-NEXT: 0 | void Test8::Z::f()
+  // TEST8-Z-NEXT: 1 | void A::z()
+
+  // TEST8-Z: VFTable indices for 'Test8::Z' (1 entries).
+  // TEST8-Z-NEXT: via vbtable index 1, vfptr at offset 0
+  // TEST8-Z-NEXT: 0 | void Test8::Z::f()
+  virtual void f();
+};
+Z z;
 }
 
 namespace Test9 {