JonasToth added a comment.
Herald added subscribers: wuzish, whisperity.
You need tests for this check. Please harden them against macro-interference as
well. I think the transformation should happen as function call that return
`Optional` and if any failure happens the diagnostic can still be emitted.
================
Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp:27
+void ReplaceMemcpyByStdCopy::registerMatchers(MatchFinder *Finder) {
+ assert(Finder != nullptr);
+
----------------
this assertion is not necessary. `Finder` is always not-null.
================
Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp:33
+ auto MemcpyMatcher =
+ callExpr(hasDeclaration(functionDecl(hasName("memcpy"),
+ isExpansionInSystemHeader())),
----------------
please match on `::memcpy` and `::std::memcpy`.
================
Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp:53
+ const auto *MemcpyNode = Result.Nodes.getNodeAs<CallExpr>("memcpy_function");
+ assert(MemcpyNode != nullptr);
+
----------------
this assertion is not necessary, as you only match on one thing and that means
it exists. (otherwise everything is broken already).
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63324/new/
https://reviews.llvm.org/D63324
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
