[PATCH] Abstract out parts of thunk emission code, add support for simple thunks for -cxx-abi microsoft

[Reid Kleckner]

  Some initial comments, because I'm super excited to see this finally arrive.  :)  I'll have more later.


================
Comment at: lib/AST/MicrosoftMangle.cpp:1420
@@ -1418,2 +1419,3 @@
     switch (MD->getAccess()) {
       default:
+        llvm_unreachable("Unsupported access specifier");
----------------
Make this "case AS_none", and rely on -Wcovered-switch to ensure that we don't miss any cases.

================
Comment at: lib/AST/MicrosoftMangle.cpp:1881
@@ +1880,3 @@
+    switch (MD->getAccess()) {
+    default:
+      llvm_unreachable("Unsupported access specifier");
----------------
AS_none

================
Comment at: lib/AST/MicrosoftMangle.cpp:1897
@@ +1896,3 @@
+    switch (MD->getAccess()) {
+    default:
+      llvm_unreachable("Unsupported access specifier");
----------------
AS_none

================
Comment at: lib/AST/MicrosoftMangle.cpp:1884
@@ +1883,3 @@
+    case AS_private:
+      Out << "G";
+      break;
----------------
Nitty micro-optimization: when you have a character, just stream the character.  It's hard to optimize the string stream back the character stream.

================
Comment at: lib/AST/MicrosoftMangle.cpp:1933
@@ +1932,3 @@
+    const ThisAdjustment &Adjustment, raw_ostream &Out) {
+  // FIXME: Actually, the dtor thunk should be emitted for vector deleting
+  // dtors rather than scalar deleting dtors. Just use the vector deleting dtor
----------------
Peter Collingbourne published a patch for these many months ago that I think works.  Feel free to ping it or take it over.  I'm OK with this FIXME for now though.

================
Comment at: lib/AST/MicrosoftMangle.cpp:1942
@@ -1902,1 +1941,3 @@
+  Mangler.mangleFunctionType(DD->getType()->castAs<FunctionProtoType>(),
+                             DD, true, true);
 }
----------------
"true, true" here is gross.  The other booleans without names are also gross.  We should make up and pass in a proper enum along the lines of:
enum FunctionDeclType {
FDT_FreeFunctoin
FDT_InstanceMethod
FDT_StructorMethod
};

All structors are instance methods, so we don't need the 4th state.

Feel free to commit that cleanup change separately with post-commit review.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:189
@@ +188,3 @@
+  void setThunkLinkage(llvm::Function *Thunk, bool AvailableExternally) {
+    assert(AvailableExternally && "All thunks should be available externally");
+    Thunk->setLinkage(llvm::GlobalValue::WeakAnyLinkage);
----------------
Why?  The MS C++ ABI has no key functions, so nothing is guaranteed to be available externally, basically ever.

================
Comment at: test/CodeGenCXX/microsoft-abi-thunks.cpp:44
@@ +43,3 @@
+ protected:
+  virtual void protected_f();
+  // MANGLING-DAG: @"\01?protected_f at C@@O3AEXXZ"
----------------
Why no thunks for the protected one?

================
Comment at: lib/CodeGen/CGCXXABI.h:357
@@ -347,1 +356,3 @@
+                               bool AvailableExternally) = 0;
+
   virtual void EmitReturnFromThunk(CodeGenFunction &CGF,
----------------
These two together don't seem like the right interface.  I'll have more comments later.


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