[PATCH] Fix PR20444 -- wrong number of vftable slots created when return adjustment thunks are needed

[Timur Iskhodzhanov]

================
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