[PATCH] [ms-cxxabi] Construct and destroy call arguments in the correct order

Reid Kleckner rnk at google.com
Tue Dec 3 10:57:47 PST 2013



================
Comment at: lib/CodeGen/CGObjC.cpp:376-387
@@ -375,3 +375,14 @@
   CallArgList Args;
-  EmitCallArgs(Args, method, E->arg_begin(), E->arg_end());
+  bool AllowExtraArgs = true;
+  ObjCMethodDecl::arg_type_iterator ArgTyBeg(
+      0, ObjCMethodDecl::deref_fun(&ParmVarDecl::getType));
+  ObjCMethodDecl::arg_type_iterator ArgTyEnd(
+      0, ObjCMethodDecl::deref_fun(&ParmVarDecl::getType));
+  if (method) {
+    AllowExtraArgs = method->isVariadic();
+    ArgTyBeg = method->arg_type_begin();
+    ArgTyEnd = method->arg_type_end();
+  }
+  EmitCallArgs(Args, AllowExtraArgs, ArgTyBeg, ArgTyEnd, E->arg_begin(),
+               E->arg_end());
 
----------------
Richard Smith wrote:
> This change looks like it would be redundant if you turned the old form of `EmitCallArgs` back into a template.
I didn't like that this had to be a template due to the one-off case of ObjCMethodDecl, so I was kind of attached to the idea of only instantiating it with FPT::arg_type_iterator in the CGCall.cpp and CGObjC.cpp TUs.  Doesn't matter that much, though.

================
Comment at: lib/CodeGen/CGCall.cpp:1460
@@ +1459,3 @@
+  if (getTarget().getCXXABI().areArgsDestroyedLeftToRightInCallee()) {
+    for (unsigned i = Args.size() - 1, e = ~0U; i != e; --i)
+      EmitParmDecl(*Args[i], ArgVals[i], i + 1);
----------------
Timur Iskhodzhanov wrote:
> how about
> 
>   for (unsigned i = Args.size(); i != 0; --i)
>     EmitParmDecl(*Args[i - 1], ArgVals[i - 1], i);
> 
> ?
After mentioning it at lunch yesterday, I like 'int i... i >= 0' best.

================
Comment at: lib/CodeGen/CGBlocks.cpp:961-970
@@ -960,4 +960,12 @@
   // And the rest of the arguments.
-  EmitCallArgs(Args, FnType->getAs<FunctionProtoType>(),
-               E->arg_begin(), E->arg_end());
+  bool AllowExtraArgs = true;
+  FunctionProtoType::arg_type_iterator ArgTyBeg = 0, ArgTyEnd = 0;
+  const FunctionProtoType *FPT = FnType->getAs<FunctionProtoType>();
+  if (FPT) {
+    AllowExtraArgs = FPT->isVariadic();
+    ArgTyBeg = FPT->arg_type_begin();
+    ArgTyEnd = FPT->arg_type_end();
+  }
+  EmitCallArgs(Args, AllowExtraArgs, ArgTyBeg, ArgTyEnd, E->arg_begin(),
+               E->arg_end());
 
----------------
Richard Smith wrote:
> This change looks unnecessary.
Fixed, missed it previously.

================
Comment at: lib/CodeGen/CGCall.cpp:2049
@@ +2048,3 @@
+    size_t CallArgsStart = Args.size();
+    for (int I = ArgTypes.size() - 1; I >= 0; --I) {
+      CallExpr::const_arg_iterator Arg = ArgBeg + I;
----------------
Timur Iskhodzhanov wrote:
> this isn't consistent with the other loops
Leaving as is.


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



More information about the cfe-commits mailing list