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

Reid Kleckner rnk at google.com
Mon Dec 16 14:03:32 PST 2013



================
Comment at: lib/CodeGen/CGClass.cpp:1699
@@ +1698,3 @@
+  // manner.
+  int ExtraArgs = CGM.getCXXABI().addImplicitConstructorArgs(
+      *this, D, Type, ForVirtualBase, Delegating, Args, ArgBeg, ArgEnd);
----------------
Timur Iskhodzhanov wrote:
> 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"?
Hah, I just hadn't finished the refactoring.  I renamed it to what I wanted but didn't finish it.

================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:926
@@ +925,3 @@
+  const FunctionProtoType *FPT = D->getType()->castAs<FunctionProtoType>();
+  CGF.EmitCallArgs(Args, FPT, ArgBeg, ArgEnd);
+  return ExtraArgs;
----------------
Timur Iskhodzhanov wrote:
> 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`.
Yep, that was the idea.

================
Comment at: test/CodeGenCXX/microsoft-abi-structors.cpp:309
@@ +308,3 @@
+  // MSVC ignores explicit CCs on constructors.
+  //__cdecl B(char a);
+};
----------------
Timur Iskhodzhanov wrote:
> why put it into the test?
It's a logical thing to test, but our output doesn't match MSVC because it ignores the CC with a warning, so I didn't finish the test.  I'll finish it with a FIXME.


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



More information about the cfe-commits mailing list