================
Comment at: lib/AST/VTableBuilder.cpp:2795
@@ -2794,3 @@
-/// methods.
-bool VFTableBuilder::NeedsReturnAdjustingThunk(const CXXMethodDecl *MD) {
- OverriddenMethodsSetTy OverriddenMethods;
----------------
Reid Kleckner wrote:
> So this fix (I think I wrote it?) was incorrect. We need to look back at the
> chain of method overrides in the current vftable, rather than all possible
> overrides.
Yep, you wrote it in r198080.
It was pretty much as incorrect as the earlier version that I've written :)
We just had insufficient test coverage to tell if we were right...
================
Comment at: lib/AST/VTableBuilder.cpp:2885-2886
@@ +2884,4 @@
+ // convertible to the OverriddenMD's one.
+ // Interestingly, a new vftable slot is also created if OverriddenMD
+ // itself required an extra slot due to return adjustment.
+ ReturnAdjustingThunk = !ComputeReturnAdjustmentBaseOffset(
----------------
Reid Kleckner wrote:
> I would say something like:
> // Once a chain of method overrides adds a return adjusting vftable slot,
> all subsequent overrides will also use an extra method slot.
>
> Would you agree with that statement?
Yes, this statement seems to be correct. In my comment I wanted to emphasize
that we have to do it even if this is suboptimal vftable-size-wise, but
apparently made the main point a bit too complex.
Do you think I should just use your wording or merge the two?
http://reviews.llvm.org/D4822
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
