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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits