Ah, I see. This turned out to be easier than I thought, so here we go: http://llvm-reviews.chandlerc.com/D2120
2013/11/5 Reid Kleckner <[email protected]>: > > > ================ > Comment at: lib/AST/VTableBuilder.cpp:3314-3316 > @@ +3313,5 @@ > + const ASTRecordLayout &BaseLayout = Context.getASTRecordLayout(CurBase); > + if (!BaseLayout.hasVBPtr() || > + DerivedVBPtrOffset != BaseOffset + BaseLayout.getVBPtrOffset()) > + break; > + > ---------------- > Timur Iskhodzhanov wrote: >> Reid Kleckner wrote: >> > I think this can just be: >> > >> > if (CurBase->getNumVBases() == 0) >> > continue; >> > >> > No need to get the ASTRecordLayout of the nvbases. >> Err, no - it leads to wrong getVBTableIndex() return values which result in >> later assertion failures in the VBTableBuilder. >> >> After some thought, I've decided to bring back "continue;" rather than >> "break;". >> This makes this code work correctly regardless of the order of nvbases >> returned by bases_begin()/end(). >> I don't think this orded is defined in such a way that the derived class is >> guaranteed to share its vbptr with first nvbase with vbases... >> See the test I've added in the new iteration. It happens to work correctly >> now, but I'm not sure it's guaranteed to work with "break;" in the future. >> >> I don't think we really want to optimize this way too much as the results >> are cached and there shouldn't be many classes with many nvbases. > I think the problem with my suggestion is the case you identified: when the > primary base is the second nvbase, we probably override its vbtable instead. > > I don't think comparing offsets is a reliable, clean, or understandable way > to identify which nvbase holds our vbtable, something which record layout > already figured out for us. I'm not really concerned with performance. > > We need something in ASTRecordLayout similar to PrimaryBase but for vbtables. > Probably BaseWithVBTable. > > > http://llvm-reviews.chandlerc.com/D2089 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
