[PATCH] D143342: [clang-tidy] Support std::format and std::print in readability-redundant-string-cstr
Mike Crowe via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 15 08:40:04 PST 2023
mikecrowe added a comment.
Thanks for the review and further suggestions.
Mike.
================
Comment at: clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp:181-190
+
+ // Detect redundant 'c_str()' calls in parameters passed to std::print and
+ // std::format.
+ Finder->addMatcher(
+ traverse(
+ TK_AsIs,
+ callExpr(
----------------
njames93 wrote:
> Can this be wrapped in a check to make sure it only runs in c++20
>
> Likewise `::std::print` is only c++23, so maybe:
> ```lang=c++
> getLangOpts().CPlusPlus2B ? hasAnyName("::std::print", "::std::format") : hasAnyName("::std::format")
> ```
I can do that. I had (presumably incorrectly) assumed that since and use of `std::print` and `std::format` in any previous version would be UB then it was safe to apply the check regardless of the C++ version.
================
Comment at: clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp:1
-// RUN: %check_clang_tidy %s readability-redundant-string-cstr %t
+// RUN: %check_clang_tidy -std=c++20 %s readability-redundant-string-cstr %t
----------------
njames93 wrote:
> We shouldn't be removing tests from older language standards
> Can all the changes made to the file be moved into a new file `redundant-string-cstr-cpp20.cpp`
> Would likely either need to create a second file for c++23 or make use of the check-suffix to run the tests for std::print
My brief reading up on check-suffix implies that it could be used to keep everything in `redundant-string-cstr.cpp`, but that file is getting rather large anyway. Having `redundant-string-cstr-cpp20.cpp` also containing C++23 tests might be a bit confusing. Having `redundant-string-cstr-cpp23.cpp` too would mean moving the format_args stuff to a header (which might be beneficial regardless.)
Anyway, I shall have a play and see what I can make work.
Do I need to add tests to ensure that the checks don't trigger when running against earlier standard versions?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143342/new/
https://reviews.llvm.org/D143342
More information about the cfe-commits
mailing list