[cfe-commits] r100940 - in /cfe/trunk: lib/AST/RecordLayoutBuilder.cpp test/CodeGenCXX/vtable-layout.cpp

[Anders Carlsson]

Author: andersca
Date: Sat Apr 10 13:42:27 2010
New Revision: 100940

URL: http://llvm.org/viewvc/llvm-project?rev=100940&view=rev
Log:
Simplify the virtual base layout code and fix a bug where we wouldn't store the offset for a virtual base.

Modified:
    cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
    cfe/trunk/test/CodeGenCXX/vtable-layout.cpp

Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=100940&r1=100939&r2=100940&view=diff
==============================================================================
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Sat Apr 10 13:42:27 2010
@@ -170,6 +170,9 @@
       // We have a virtual primary base, insert it as an indirect primary base.
       IndirectPrimaryBases.insert(Base);
 
+      assert(!VisitedVirtualBases.count(Base) && "vbase already visited!");
+      VisitedVirtualBases.insert(Base);
+      
       LayoutVirtualBase(Base);
     } else
       LayoutNonVirtualBase(Base);
@@ -209,12 +212,23 @@
                                            uint64_t Offset,
                                            const CXXRecordDecl *MostDerivedClass) {
   const CXXRecordDecl *PrimaryBase;
+  bool PrimaryBaseIsVirtual;
 
-  if (MostDerivedClass == RD)
+  if (MostDerivedClass == RD) {
     PrimaryBase = this->PrimaryBase.getBase();
-  else {
+    PrimaryBaseIsVirtual = this->PrimaryBase.isVirtual();
+  } else {
     const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(RD);
     PrimaryBase = Layout.getPrimaryBase();
+    PrimaryBaseIsVirtual = Layout.getPrimaryBaseWasVirtual();
+  }
+
+  // Check the primary base first.
+  if (PrimaryBase && PrimaryBaseIsVirtual && 
+      VisitedVirtualBases.insert(PrimaryBase)) {
+    assert(!VBases.count(PrimaryBase) && "vbase offset already exists!");
+    
+    VBases.insert(std::make_pair(PrimaryBase, Offset));
   }
 
   for (CXXRecordDecl::base_class_const_iterator I = RD->bases_begin(),
@@ -226,28 +240,15 @@
       cast<CXXRecordDecl>(I->getType()->getAs<RecordType>()->getDecl());
 
     if (I->isVirtual()) {
-      bool IndirectPrimaryBase = IndirectPrimaryBases.count(Base);
+      if (PrimaryBase != Base || !PrimaryBaseIsVirtual) {
+        bool IndirectPrimaryBase = IndirectPrimaryBases.count(Base);
 
-      // We only want to visit this virtual base if it's either a primary base,
-      // or not an indirect primary base.
-      if (Base == PrimaryBase || !IndirectPrimaryBase) {
-        // Only lay things out once.
-        if (!VisitedVirtualBases.insert(Base))
-          continue;
-
-        if (Base == PrimaryBase) {
-          assert(IndirectPrimaryBase &&
-                 "Base is supposed to be an indirect primary base!");
-
-          // We only want to add a vbase offset if this primary base is not the
-          // primary base of the most derived class.
-          if (PrimaryBase != this->PrimaryBase.getBase() ||
-              !this->PrimaryBase.isVirtual()) {
-            if (!VBases.insert(std::make_pair(Base, Offset)).second)
-              assert(false && "Added same vbase offset more than once!");
-          }
-        } else {
-          // We actually do want to lay out this base.
+        // Only lay out the virtual base if it's not an indirect primary base.
+        if (!IndirectPrimaryBase) {
+          // Only visit virtual bases once.
+          if (!VisitedVirtualBases.insert(Base))
+            continue;
+          
           LayoutVirtualBase(Base);
         }
       }

Modified: cfe/trunk/test/CodeGenCXX/vtable-layout.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/vtable-layout.cpp?rev=100940&r1=100939&r2=100940&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/vtable-layout.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/vtable-layout.cpp Sat Apr 10 13:42:27 2010
@@ -1303,3 +1303,26 @@
 V2 *B::f() { return 0; }
 
 }
+
+namespace Test30 {
+
+// Test that we don't assert when generating a vtable for F.
+struct A { };
+
+struct B : virtual A {
+ int i;
+};
+
+struct C {
+ virtual void f();
+};
+
+struct D : virtual C, B { };
+struct E : virtual D { };
+
+struct F : E {
+ virtual void f();
+};
+void F::f() { }
+
+}