[PATCH] [-cxx-abi microsoft] Emit thunks for pointers to virtual member functions

[Reid Kleckner]

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1411-1412
@@ +1410,4 @@
+      SmallString<256> ThunkName;
+      bool Is64Bit = getContext().getTargetInfo().getPointerWidth(0) == 64;
+      int OffsetInVFTable = ML.Index * (Is64Bit ? 8 : 4);
+      llvm::raw_svector_ostream Out(ThunkName);
----------------
IMO the right way to write this is something like: 

  ML.Index * getContext().getTypeSizeInChars(getContext().VoidPtrTy)

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1413-1415
@@ +1412,5 @@
+      int OffsetInVFTable = ML.Index * (Is64Bit ? 8 : 4);
+      llvm::raw_svector_ostream Out(ThunkName);
+      getMangleContext().mangleVirtualMemPtrThunk(MD, OffsetInVFTable, Out);
+      Out.flush();
+
----------------
Maybe it's cleaner to pass the index to EmitVirtualMemPtrThunk and let it mangle?

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1964-1965
@@ +1963,4 @@
+
+  // Note: we don't do any return adjustment. The callee should be doing that if
+  // necessary.
+
----------------
We don't need return adjustment because we don't have to worry about converting member function pointers that return covariant return types:

  struct A { int a; };
  struct B { int b; };
  struct C : public A, public B { int c; };
  C *covariant_return();
  B *(*fptr)();
  int main() {
    fptr = &covariant_return; // expected-error {{incompatible type}}
  }

The same logic applies to pointers to member functions.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1972
@@ +1971,3 @@
+  CGF.FinishFunction();
+  CGM.setFunctionLinkage(MD, ThunkFn); // XXX: Do we need this?
+
----------------
Timur Iskhodzhanov wrote:
> Isn't this copying the function's linkage over to the thunk?
> Don't you get "symbol already defined" if you link two TUs generating the same thunk together?
These should be merged like inline functions.  They should be linkonce_odr for externally visible types and internal for internal types.  We probably have logic for that somewhere.


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