[PATCH] D54900: [Sema] Avoid CallExpr::setNumArgs in Sema::BuildCallToObjectOfClassType

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno created this revision.
riccibruno added reviewers: rsmith, aaron.ballman.
riccibruno added a project: clang.
Herald added subscribers: cfe-commits, dexonsmith, inglorion, mehdi_amini.

`CallExpr::setNumArgs` is the only thing that prevents storing the arguments
of a call expression in a trailing array since it might resize the argument array.
`setNumArgs` is only called in 3 places in `Sema`, and for all of them it is
possible to avoid it.

This deals with the call to `setNumArgs` in `BuildCallToObjectOfClassType`.
Instead of constructing the `CXXOperatorCallExpr` first and later calling
`setNumArgs` if we have default arguments, we first construct a large
enough `SmallVector`, do the promotion/check of the arguments, and
then construct the `CXXOperatorCallExpr`.

Incidentally this also avoid reallocating the arguments when the
call operator has default arguments but this is not the primary goal.


Repository:
  rC Clang

https://reviews.llvm.org/D54900

Files:
  lib/Sema/SemaOverload.cpp


Index: lib/Sema/SemaOverload.cpp
===================================================================
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -13257,29 +13257,14 @@
   if (NewFn.isInvalid())
     return true;
 
+  // The number of arguments slots to allocate in the call.
+  // If we have default arguments we need to allocate space for them
+  // as well. We additionally need one more slot for the object parameter.
+  unsigned NumArgsSlots = 1 + std::max<unsigned>(Args.size(), NumParams);
+
   // Build the full argument list for the method call (the implicit object
   // parameter is placed at the beginning of the list).
-  SmallVector<Expr *, 8> MethodArgs(Args.size() + 1);
-  MethodArgs[0] = Object.get();
-  std::copy(Args.begin(), Args.end(), MethodArgs.begin() + 1);
-
-  // Once we've built TheCall, all of the expressions are properly
-  // owned.
-  QualType ResultTy = Method->getReturnType();
-  ExprValueKind VK = Expr::getValueKindForType(ResultTy);
-  ResultTy = ResultTy.getNonLValueExprType(Context);
-
-  CXXOperatorCallExpr *TheCall = new (Context)
-      CXXOperatorCallExpr(Context, OO_Call, NewFn.get(), MethodArgs, ResultTy,
-                          VK, RParenLoc, FPOptions());
-
-  if (CheckCallReturnType(Method->getReturnType(), LParenLoc, TheCall, Method))
-    return true;
-
-  // We may have default arguments. If so, we need to allocate more
-  // slots in the call for them.
-  if (Args.size() < NumParams)
-    TheCall->setNumArgs(Context, NumParams + 1);
+  SmallVector<Expr *, 8> MethodArgs(NumArgsSlots);
 
   bool IsError = false;
 
@@ -13291,7 +13276,7 @@
     IsError = true;
   else
     Object = ObjRes;
-  TheCall->setArg(0, Object.get());
+  MethodArgs[0] = Object.get();
 
   // Check the argument types.
   for (unsigned i = 0; i != NumParams; i++) {
@@ -13320,7 +13305,7 @@
       Arg = DefArg.getAs<Expr>();
     }
 
-    TheCall->setArg(i + 1, Arg);
+    MethodArgs[i+1] = Arg;
   }
 
   // If this is a variadic call, handle args passed through "...".
@@ -13330,14 +13315,27 @@
       ExprResult Arg = DefaultVariadicArgumentPromotion(Args[i], VariadicMethod,
                                                         nullptr);
       IsError |= Arg.isInvalid();
-      TheCall->setArg(i + 1, Arg.get());
+      MethodArgs[i+1] = Arg.get();
     }
   }
 
-  if (IsError) return true;
+  if (IsError)
+    return true;
 
   DiagnoseSentinelCalls(Method, LParenLoc, Args);
 
+  // Once we've built TheCall, all of the expressions are properly owned.
+  QualType ResultTy = Method->getReturnType();
+  ExprValueKind VK = Expr::getValueKindForType(ResultTy);
+  ResultTy = ResultTy.getNonLValueExprType(Context);
+
+  CXXOperatorCallExpr *TheCall = new (Context)
+      CXXOperatorCallExpr(Context, OO_Call, NewFn.get(), MethodArgs, ResultTy,
+                          VK, RParenLoc, FPOptions());
+
+  if (CheckCallReturnType(Method->getReturnType(), LParenLoc, TheCall, Method))
+    return true;
+
   if (CheckFunctionCall(Method, TheCall, Proto))
     return true;
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D54900.175247.patch
Type: text/x-patch
Size: 3047 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181126/6e0facc8/attachment.bin>