[PATCH] D54902: [AST][Sema] Remove CallExpr::setNumArgs

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM aside from two small NFC nits.



================
Comment at: include/clang/AST/ExprCXX.h:168
 public:
-  CXXMemberCallExpr(ASTContext &C, Expr *fn, ArrayRef<Expr*> args,
-                    QualType t, ExprValueKind VK, SourceLocation RP)
-      : CallExpr(C, CXXMemberCallExprClass, fn, args, t, VK, RP) {}
+  CXXMemberCallExpr(ASTContext &C, Expr *fn, ArrayRef<Expr *> args, QualType t,
+                    ExprValueKind VK, SourceLocation RP,
----------------
riccibruno wrote:
> aaron.ballman wrote:
> > Since you're already touching the line, can you correct the names `fn`, `args`, and `t` to match our naming conventions?
> I was going to submit an NFC cleaning up all of `CallExpr` +
> the classes deriving from it in one go (capitalization style,
> `clang-format`, and so on).
> 
> I would prefer to avoid mixing style fixes in existing code
> and functional changes.
That's fine by me, thanks!


================
Comment at: lib/Sema/SemaExpr.cpp:5607
-      Fn = rewrite.get();
-      TheCall->setCallee(Fn);
-      goto retry;
----------------
riccibruno wrote:
> aaron.ballman wrote:
> > Why did this go away?
> Because it is now set by the constructor of `CallExpr` or
> `CUDAKernelCallExpr`. The call to `setCallee` was needed
> before because the call expression was constructed before
> this piece of code, but now we can just pass `Fn` to the
> constructor.
Thank you for the explanation, that makes sense to me.


================
Comment at: lib/Sema/SemaExpr.cpp:5625
 
   // Bail out early if calling a builtin with custom typechecking.
   if (BuiltinID && Context.BuiltinInfo.hasCustomTypechecking(BuiltinID))
----------------
typechecking -> type checking


================
Comment at: lib/Serialization/ASTReaderStmt.cpp:2499
+      S = new (Context) CallExpr(
+          Context, /* NumArgs=*/Record[ASTStmtReader::NumExprFields + 0],
+          Empty);
----------------
I would drop the `+ 0` from all of these; they look a bit like a typo from a casual reading of the code.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54902/new/

https://reviews.llvm.org/D54902