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

[Timur Iskhodzhanov]

I've decided this change is trivial enough to do post-commit review.

FTR, the FindDirectlyOverriddenMethodInBases function was different
from FindNearestOverriddenMethod just because I didn't quite
understand what is returned by {begin,end}_overridden_methods() in
cases like Test8::Z.

2013/10/29 Timur Iskhodzhanov <timurrrr at google.com>:
> 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 {
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits