[PATCH] D57948: [Sema] Fix a regression introduced in "[AST][Sema] Remove CallExpr::setNumArgs"

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 11 05:37:04 PST 2019


aaron.ballman added inline comments.


================
Comment at: lib/AST/Expr.cpp:1272-1274
+  for (unsigned I = NumArgs; I != NumArgsAllocated; ++I) {
+    TrailingArgs[I] = nullptr;
   }
----------------
Could use `std::fill()` here to remove the for-loop. Your call whether that looks cleaner or not.


================
Comment at: lib/Sema/SemaExpr.cpp:5810
+    // However it may not have been constructed with enough storage. In this
+    // case rebuild the node with enough storage. The waste of space is
+    // immaterial since this only happens when some typos were corrected.
----------------
case rebuild -> case, rebuild


================
Comment at: lib/Sema/SemaOverload.cpp:12909-12910
 
+    call->setNumArgsUnsafe(
+        std::max<unsigned>(Args.size(), proto->getNumParams()));
     if (ConvertArgumentsForCall(call, op, nullptr, proto, Args, RParenLoc))
----------------
Oye. When we had to call this hack in one place, I could hold my nose, but having to call it randomly any time something like a call expression is created is a bit too much for me.

How awful is the alternative fix, such as making things resilient to null parameters in `BuildResolvedCallExpr()`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57948





More information about the cfe-commits mailing list