[PATCH] Fix vbtable indices if a class shares the vbptr with a non-virtual base

Reid Kleckner rnk at google.com
Fri Nov 1 13:07:23 PDT 2013


  Do you think it's worth building some kind of layout data structure for this, or should we just recompute it on every query?  If we did make a layout structure, I'd work it into computeVTableRelatedInformation().  If we do that, are you still sure you want to call it MicrosoftVFTableContext?  I remember wanting to call it MicrosoftVTableContext so we could put vbtable stuff in there too.


================
Comment at: lib/AST/VTableBuilder.cpp:2415-2416
@@ +2414,4 @@
+              BaseOffset = Layout.getBaseClassOffset(CurBase);
+    if (DerivedVBPtrOffset < BaseOffset)
+      continue;
+    const ASTRecordLayout &BaseLayout = Context.getASTRecordLayout(CurBase);
----------------
Can this happen?  I thought we always just used the first nvbase with vbases.  If so, I'd make that the loop exit criteria, since it's simpler than this vbptr offset comparison.

================
Comment at: lib/CodeGen/MicrosoftVBTables.cpp:213
@@ -213,2 +212,3 @@
     Offset -= VBPtrSubobject.getBaseOffset() + VBPtrOffset;
-    Offsets.push_back(llvm::ConstantInt::get(CGM.IntTy, Offset.getQuantity()));
+    llvm::Constant *&VBOffset = Offsets[GetVBTableIndex(ReusingBase, VBase)];
+    assert(VBOffset == 0 && "The same vbindex seen twice?");
----------------
This is silly, we're walking the inheritance graph in a loop.  Make something that returns the full BaseSetVectorTy, and we can use that to fill in this table.


http://llvm-reviews.chandlerc.com/D2089



More information about the cfe-commits mailing list