[PATCH] D107450: [clang-tidy] Fix wrong FixIt in performance-move-const-arg

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp:149-151
+      const auto *ReceivingCallExpr = dyn_cast<CallExpr>(ReceivingExpr);
+      const auto *ReceivingConstructExpr =
+          dyn_cast<CXXConstructExpr>(ReceivingExpr);
----------------
It looks like `ReceivingExpr` can be null (see line 78 that you added), so the first `dyn_cast` will crash if given null. Further, if the result of the first dynamic cast is null, then the second `dyn_cast` will also crash.

Should there be a null pointer check? Should we use `cast` instead of `dyn_cast` for the first one?


================
Comment at: clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp:153
+      if ((!ReceivingCallExpr ||
+           ReceivingCallExpr->getDirectCallee()->isTemplateInstantiation()) &&
+          (!ReceivingConstructExpr ||
----------------
`getDirectCallee()` can return null.


================
Comment at: clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp:157-163
+      const NamedDecl *FunctionName = nullptr;
+      if (ReceivingCallExpr)
+        FunctionName =
+            ReceivingCallExpr->getDirectCallee()->getUnderlyingDecl();
+      else
+        FunctionName =
+            ReceivingConstructExpr->getConstructor()->getUnderlyingDecl();
----------------
`getDirectCallee()` can return nullptr.


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

https://reviews.llvm.org/D107450