njames93 requested changes to this revision. njames93 added inline comments. This revision now requires changes to proceed.
================ Comment at: clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp:186-187 + traverse(TK_AsIs, + callExpr(anyOf(callee(functionDecl(hasName("::std::print"))), + callee(functionDecl(hasName("::std::format")))), + hasAnyArgument(materializeTemporaryExpr( ---------------- ================ Comment at: clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp:188 + callee(functionDecl(hasName("::std::format")))), + hasAnyArgument(materializeTemporaryExpr( + has(StringCStrCallExpr))))), ---------------- The limitation about only transforming the first argument can be alleviated by using the `forEachArgumentWithParam` matcher ================ Comment at: clang-tools-extra/docs/ReleaseNotes.rst:129-131 + arguments to ``std::print`` and ``std::format``. Note that only the first + such argument is currently reported so it may be necessary to run the + check multiple times to fix all of them. ---------------- Please address this needless restriction, see above comment ================ Comment at: clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp:336 + // Ideally we'd fix both the second and fourth parameters here, but that doesn't work. + auto r2 = std::format("One:{} Two:{} Three:{}\n", s1.c_str(), s2, s3.c_str()); + // CHECK-MESSAGES: :[[@LINE-1]]:53: warning: redundant call to 'c_str' [readability-redundant-string-cstr] ---------------- Again this test case would need updating to handle applying all fixes CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143342/new/ https://reviews.llvm.org/D143342 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits