[PATCH] Add MicrosoftVFTableContext to AST

Reid Kleckner rnk at google.com
Thu Jul 11 09:31:49 PDT 2013


  LGTM, but John should review it.


================
Comment at: lib/AST/VTableBuilder.cpp:2562
@@ +2561,3 @@
+
+    for (CXXRecordDecl::base_class_const_iterator I = RD->bases_begin(),
+          E = RD->bases_end(); I != E; ++I) {
----------------
Timur Iskhodzhanov wrote:
> Reid Kleckner wrote:
> > Timur Iskhodzhanov wrote:
> > > Reid Kleckner wrote:
> > > > Can you explain why we can't use the base subobjects from FinalOverriders to get a list of all BaseSubobjects?  Then we should be able to find the base with the smallest offset.
> > > This might be possible, but I haven't found a simple way to do that... It's not "all subobjects" but rather "all the subobjects that have the same method declared", and even a bit more complex than that (see the O_o test for the reordered virtual bases).
> > I think I understand what this is doing now.  Would this algorithm be simpler (no recursion) and do the same thing?
> > 
> > Call ComputeAllOverriddenMethods to get the set of all overridden methods.
> > Compute the set of bases in which those methods are declared. (getParent loop)
> > Call MostDerived->lookupInBases(BaseInSet, &BaseSet, &Paths)
> > For all base paths, walk the path to compute the offsets using the funky logic for virtual bases
> > Pick the minimum offset
> > 
> > This could probably be improved to avoid the lookupInBases inside a loop over every virtual method decl, but that's more of a FIXME level thing.
> Wow, it worked!
> Can't say the code is that shorter but yeah, it's definitely a bit easier to read.
Nice!


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



More information about the cfe-commits mailing list