================ 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 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
