[PATCH] [ms-cxxabi] The "most derived" ctor parameter comes last

Reid Kleckner rnk at google.com
Fri Dec 13 10:53:27 PST 2013



================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:825
@@ -812,1 +824,3 @@
+  CGF.EmitCall(CGM.getTypes().arrangeCXXMethodCall(Args, FPT, required), Callee,
+               ReturnValueSlot(), Args, D);
 }
----------------
Timur Iskhodzhanov wrote:
> Hm, I don't like duplication code here.
> In my version of the patch, I've added something like a `bool ImplicitParameterFollowsExplicit` parameter to EmitCXXMemberCall.
> 
> What do you think?
I'm not really happy with the proliferation of boolean parameters like ForVirtualBase and Delegating below.  I'd rather explicitly build up the CallArgList in the MS ABI code.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:798
@@ -801,3 +797,3 @@
 
-  llvm::Value *ImplicitParam = 0;
-  QualType ImplicitParamTy;
+  // C++11 [class.mfct.non-static]p2:
+  //   If a non-static member function of a class X is called for an object that
----------------
Timur Iskhodzhanov wrote:
> I think this deserves a separate patch.
> Also, why is it MS ABI specific?
This comes from EmitCXXMemberCall.

================
Comment at: lib/CodeGen/CodeGenFunction.h:2495
@@ -2494,2 +2494,3 @@
 
+public:
   /// EmitCallArgs - Emit call arguments for a function.
----------------
Timur Iskhodzhanov wrote:
> I don't like changing the public interface here, see my suggestion below.
Why not?  It's only available in CodeGen anyway, and CGF is a pretty public class that is tightly coupled with the CGCXXABI subclasses.

================
Comment at: lib/CodeGen/CodeGenFunction.cpp:697
@@ -696,3 +696,3 @@
   CurGD = GD;
-  const CXXMethodDecl *MD;
+  const CXXMethodDecl *MD = 0;
   if ((MD = dyn_cast<CXXMethodDecl>(FD)) && MD->isInstance()) {
----------------
Timur Iskhodzhanov wrote:
> `= 0` is not needed here.
My thinking was that not having it is too subtle, and the store will be trivially dead-store-eliminated.  I'll raise the MD assignment out of the if instead.


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



More information about the cfe-commits mailing list