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

[Timur Iskhodzhanov]

================
Comment at: lib/CodeGen/CGCXXABI.h:273
@@ +272,3 @@
+
+  /// Insert any ABI-specific parameters into the parameter list for a function.
+  /// This generally involves extra data for constructors and destructors.
----------------
ABI-specific implicit parameters?

================
Comment at: lib/CodeGen/CGCXXABI.h:271
@@ +270,3 @@
+  /// Build a parameter variable suitable for 'this'.
+  void BuildThisParam(CodeGenFunction &CGF, FunctionArgList &Params);
+
----------------
buildThisParam?

================
Comment at: lib/CodeGen/CGCXXABI.h:279
@@ -279,4 +278,3 @@
   /// the formal return type of the function otherwise.
-  virtual void BuildInstanceFunctionParams(CodeGenFunction &CGF,
-                                           QualType &ResTy,
-                                           FunctionArgList &Params) = 0;
+  virtual void AddImplicitStructorParams(CodeGenFunction &CGF, QualType &ResTy,
+                                         FunctionArgList &Params) = 0;
----------------
addImplicitStructorParams ?

================
Comment at: lib/CodeGen/CGCXXABI.h:292
@@ -293,2 +291,3 @@
 
   /// Emit the constructor call.
+  virtual int
----------------
The comment is out of date.
Also, see below for naming vs what it actually does.

Returns what?
Maybe use `unsigned` rather than `int` ?

================
Comment at: lib/CodeGen/CGClass.cpp:1699
@@ +1698,3 @@
+  // manner.
+  int ExtraArgs = CGM.getCXXABI().addImplicitConstructorArgs(
+      *this, D, Type, ForVirtualBase, Delegating, Args, ArgBeg, ArgEnd);
----------------
I think this is really confusing.
The method is named "add implicit ... args" but it actually adds explicit args as well, right?

Should be either "add only implicit args" or "add all args except for this" ? Or maybe just "add all args"?

================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:870
@@ -872,3 +869,3 @@
   const CXXMethodDecl *MD = cast<CXXMethodDecl>(CGF.CurGD.getDecl());
   assert(MD->isInstance());
 
----------------
  assert(isa<Constructor>(MD) || isa<Destructor>(MD));
?

================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:926
@@ +925,3 @@
+  const FunctionProtoType *FPT = D->getType()->castAs<FunctionProtoType>();
+  CGF.EmitCallArgs(Args, FPT, ArgBeg, ArgEnd);
+  return ExtraArgs;
----------------
This line has to be called for any ABI, right?
Why not put it before the `addImplicitConstructorArgs` call?

That'd also make the `insert(begin() + 1, ...)` call consistent between `BuildConstructorSignature`, `AddImplicitStructorParams` and `addImplicitConstructorArgs`.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:457
@@ -458,1 +456,3 @@
+  // All parameters are already in place except is_most_derived, which goes
+  // second if it's variadic and last if it's not.
 
----------------
s/second/right after 'this'/ ?

================
Comment at: test/CodeGenCXX/microsoft-abi-structors.cpp:309
@@ +308,3 @@
+  // MSVC ignores explicit CCs on constructors.
+  //__cdecl B(char a);
+};
----------------
why put it into the test?


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