Split adding the primary virtual base offsets out into a separate pass. This fixes...

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@101373 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/AST/RecordLayoutBuilder.cpp b/lib/AST/RecordLayoutBuilder.cpp
index 7c89b7b47b0de0b18d64be2493ebc15d13d7b1a9..c292834f5465be8b74b00657de6e74a1f5e3899a 100644 (file)
--- a/lib/AST/RecordLayoutBuilder.cpp
+++ b/lib/AST/RecordLayoutBuilder.cpp
@@ -207,10 +207,66 @@ void ASTRecordLayoutBuilder::LayoutNonVirtualBase(const CXXRecordDecl *RD) {
     assert(false && "Added same base offset more than once!");
 }
 
+void
+ASTRecordLayoutBuilder::AddPrimaryVirtualBaseOffsets(const CXXRecordDecl *RD, 
+                                        uint64_t Offset,
+                                        const CXXRecordDecl *MostDerivedClass) {
+  // We already have the offset for the primary base of the most derived class.
+  if (RD != MostDerivedClass) {
+    const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(RD);
+    const CXXRecordDecl *PrimaryBase = Layout.getPrimaryBase();
+
+    // If this is a primary virtual base and we haven't seen it before, add it.
+    if (PrimaryBase && Layout.getPrimaryBaseWasVirtual() &&
+        !VBases.count(PrimaryBase))
+      VBases.insert(std::make_pair(PrimaryBase, Offset));
+  }
+
+  for (CXXRecordDecl::base_class_const_iterator I = RD->bases_begin(),
+       E = RD->bases_end(); I != E; ++I) {
+    assert(!I->getType()->isDependentType() &&
+           "Cannot layout class with dependent bases.");
+    
+    const CXXRecordDecl *BaseDecl =
+      cast<CXXRecordDecl>(I->getType()->getAs<RecordType>()->getDecl());
+
+    if (!BaseDecl->getNumVBases()) {
+      // This base isn't interesting since it doesn't have any virtual bases.
+      continue;
+    }
+    
+    // Compute the offset of this base.
+    uint64_t BaseOffset;
+    
+    if (I->isVirtual()) {
+      // If we don't know this vbase yet, don't visit it. It will be visited
+      // later.
+      if (!VBases.count(BaseDecl)) {
+        continue;
+      }
+      
+      // Check if we've already visited this base.
+      if (!VisitedVirtualBases.insert(BaseDecl))
+        continue;
+
+      // We want the vbase offset from the class we're currently laying out.
+      BaseOffset = VBases[BaseDecl];
+    } else if (RD == MostDerivedClass) {
+      // We want the base offset from the class we're currently laying out.
+      assert(Bases.count(BaseDecl) && "Did not find base!");
+      BaseOffset = Bases[BaseDecl];
+    } else {
+      const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(RD);
+      BaseOffset = Offset + Layout.getBaseClassOffset(BaseDecl);
+    }
+
+    AddPrimaryVirtualBaseOffsets(BaseDecl, BaseOffset, MostDerivedClass);
+  }
+}
+
 void
 ASTRecordLayoutBuilder::LayoutVirtualBases(const CXXRecordDecl *RD,
-                                           uint64_t Offset,
-                                           const CXXRecordDecl *MostDerivedClass) {
+                                        const CXXRecordDecl *MostDerivedClass) {
   const CXXRecordDecl *PrimaryBase;
   bool PrimaryBaseIsVirtual;
 
@@ -223,14 +279,6 @@ ASTRecordLayoutBuilder::LayoutVirtualBases(const CXXRecordDecl *RD,
     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(),
          E = RD->bases_end(); I != E; ++I) {
     assert(!I->getType()->isDependentType() &&
@@ -259,27 +307,7 @@ ASTRecordLayoutBuilder::LayoutVirtualBases(const CXXRecordDecl *RD,
       continue;
     }
 
-    // Compute the offset of this base.
-    uint64_t BaseOffset;
-
-    if (I->isVirtual()) {
-      // If we don't know this vbase yet, don't visit it. It will be visited
-      // later.
-      if (!VBases.count(Base))
-        continue;
-  
-      // We want the vbase offset from the class we're currently laying out.
-      BaseOffset = VBases[Base];
-    } else if (RD == MostDerivedClass) {
-      // We want the base offset from the class we're currently laying out.
-      assert(Bases.count(Base) && "Did not find base!");
-      BaseOffset = Bases[Base];
-    } else {
-      const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(RD);
-      BaseOffset = Offset + Layout.getBaseClassOffset(Base);
-    }
-
-    LayoutVirtualBases(Base, BaseOffset, MostDerivedClass);
+    LayoutVirtualBases(Base, MostDerivedClass);
   }
 }
 
@@ -501,9 +529,14 @@ void ASTRecordLayoutBuilder::Layout(const RecordDecl *D) {
   NonVirtualSize = Size;
   NonVirtualAlignment = Alignment;
 
-  // If this is a C++ class, lay out its virtual bases.
-  if (RD)
-    LayoutVirtualBases(RD, 0, RD);
+  // If this is a C++ class, lay out its virtual bases and add its primary
+  // virtual base offsets.
+  if (RD) {
+    LayoutVirtualBases(RD, RD);
+
+    VisitedVirtualBases.clear();
+    AddPrimaryVirtualBaseOffsets(RD, 0, RD);
+  }
 
   // Finally, round the size of the total struct up to the alignment of the
   // struct itself.

diff --git a/lib/AST/RecordLayoutBuilder.h b/lib/AST/RecordLayoutBuilder.h
index a4bce753126a628e8f22b48317ed372338f5e52a..d0e336231c5afe9a8ef27d816f6081d5e9c19874 100644 (file)
--- a/lib/AST/RecordLayoutBuilder.h
+++ b/lib/AST/RecordLayoutBuilder.h
@@ -112,8 +112,11 @@ class ASTRecordLayoutBuilder {
   /// LayoutNonVirtualBase - Lays out a single non-virtual base.
   void LayoutNonVirtualBase(const CXXRecordDecl *RD);
 
+  void AddPrimaryVirtualBaseOffsets(const CXXRecordDecl *RD, uint64_t Offset,
+                                    const CXXRecordDecl *MostDerivedClass);
+
   /// LayoutVirtualBases - Lays out all the virtual bases.
-  void LayoutVirtualBases(const CXXRecordDecl *RD, uint64_t Offset,
+  void LayoutVirtualBases(const CXXRecordDecl *RD,
                           const CXXRecordDecl *MostDerivedClass);
 
   /// LayoutVirtualBase - Lays out a single virtual base.

diff --git a/test/CodeGenCXX/vtable-layout.cpp b/test/CodeGenCXX/vtable-layout.cpp
index 184b95818e0183c428a68b89d80b4a5d56fd5a9f..692e8deef351db9ba43ce3c9d07190c76766022c 100644 (file)
--- a/test/CodeGenCXX/vtable-layout.cpp
+++ b/test/CodeGenCXX/vtable-layout.cpp
@@ -1511,3 +1511,90 @@ struct F : E {
 void F::f() { }
 
 }
+
+namespace Test35 {
+
+// Test that we lay out the virtual bases of 'Test35::H' in the correct order.
+
+struct A {
+ virtual void a();
+
+ int i;
+};
+
+struct B : virtual A {
+ virtual void b();
+};
+
+struct C {
+ virtual void c();
+};
+
+struct D : C, virtual B {
+ virtual void d();
+};
+
+struct E : D {
+ virtual void e();
+
+ bool b;
+};
+
+struct F : virtual D { };
+struct G : virtual E { };
+
+// CHECK:      Vtable for 'Test35::H' (32 entries).
+// CHECK-NEXT:    0 | vbase_offset (32)
+// CHECK-NEXT:    1 | vbase_offset (0)
+// CHECK-NEXT:    2 | vcall_offset (0)
+// CHECK-NEXT:    3 | vcall_offset (0)
+// CHECK-NEXT:    4 | vbase_offset (16)
+// CHECK-NEXT:    5 | vbase_offset (8)
+// CHECK-NEXT:    6 | offset_to_top (0)
+// CHECK-NEXT:    7 | Test35::H RTTI
+// CHECK-NEXT:        -- (Test35::C, 0) vtable address --
+// CHECK-NEXT:        -- (Test35::D, 0) vtable address --
+// CHECK-NEXT:        -- (Test35::F, 0) vtable address --
+// CHECK-NEXT:        -- (Test35::H, 0) vtable address --
+// CHECK-NEXT:    8 | void Test35::C::c()
+// CHECK-NEXT:    9 | void Test35::D::d()
+// CHECK-NEXT:   10 | void Test35::H::h()
+// CHECK-NEXT:   11 | vbase_offset (0)
+// CHECK-NEXT:   12 | vbase_offset (24)
+// CHECK-NEXT:   13 | vcall_offset (0)
+// CHECK-NEXT:   14 | vbase_offset (8)
+// CHECK-NEXT:   15 | offset_to_top (-8)
+// CHECK-NEXT:   16 | Test35::H RTTI
+// CHECK-NEXT:        -- (Test35::B, 8) vtable address --
+// CHECK-NEXT:        -- (Test35::G, 8) vtable address --
+// CHECK-NEXT:   17 | void Test35::B::b()
+// CHECK-NEXT:   18 | vcall_offset (0)
+// CHECK-NEXT:   19 | offset_to_top (-16)
+// CHECK-NEXT:   20 | Test35::H RTTI
+// CHECK-NEXT:        -- (Test35::A, 16) vtable address --
+// CHECK-NEXT:   21 | void Test35::A::a()
+// CHECK-NEXT:   22 | vcall_offset (0)
+// CHECK-NEXT:   23 | vcall_offset (0)
+// CHECK-NEXT:   24 | vcall_offset (0)
+// CHECK-NEXT:   25 | vbase_offset (-16)
+// CHECK-NEXT:   26 | vbase_offset (-24)
+// CHECK-NEXT:   27 | offset_to_top (-32)
+// CHECK-NEXT:   28 | Test35::H RTTI
+// CHECK-NEXT:        -- (Test35::C, 32) vtable address --
+// CHECK-NEXT:        -- (Test35::D, 32) vtable address --
+// CHECK-NEXT:        -- (Test35::E, 32) vtable address --
+// CHECK-NEXT:   29 | void Test35::C::c()
+// CHECK-NEXT:   30 | void Test35::D::d()
+// CHECK-NEXT:   31 | void Test35::E::e()
+
+// CHECK:      Virtual base offset offsets for 'Test35::H' (4 entries).
+// CHECK-NEXT:    Test35::A | -32
+// CHECK-NEXT:    Test35::B | -24
+// CHECK-NEXT:    Test35::D | -56
+// CHECK-NEXT:    Test35::E | -64
+struct H : F, G {
+ virtual void h();
+};
+void H::h() { }
+
+}