https://github.com/mikecrowe updated https://github.com/llvm/llvm-project/pull/104675
>From afdd6c2a12735e1da69a46c6ff78b5d4e0dcdb6e Mon Sep 17 00:00:00 2001 From: Mike Crowe <m...@mcrowe.com> Date: Sun, 21 Jul 2024 20:37:02 +0100 Subject: [PATCH 1/2] [clang-tidy] Support member functions with modernize-use-std-print/format These checks can be made to work on member functions quite easily and it's not unknown to have at least printf-style functions as members. Let's remove the restriction. --- .../modernize/UseStdFormatCheck.cpp | 6 +- .../clang-tidy/modernize/UseStdPrintCheck.cpp | 8 +-- clang-tools-extra/docs/ReleaseNotes.rst | 6 ++ .../checks/modernize/use-std-format.rst | 5 +- .../checks/modernize/use-std-print.rst | 15 ++-- .../modernize/use-std-format-member.cpp | 44 ++++++++++++ .../modernize/use-std-print-member.cpp | 70 +++++++++++++++++++ 7 files changed, 138 insertions(+), 16 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-member.cpp create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-member.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp index 6cef21f1318a2a..cdb34aef1b0e61 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp @@ -50,8 +50,7 @@ void UseStdFormatCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( callExpr(argumentCountAtLeast(1), hasArgument(0, stringLiteral(isOrdinary())), - callee(functionDecl(unless(cxxMethodDecl()), - matchers::matchesAnyListedName( + callee(functionDecl(matchers::matchesAnyListedName( StrFormatLikeFunctions)) .bind("func_decl"))) .bind("strformat"), @@ -93,7 +92,8 @@ void UseStdFormatCheck::check(const MatchFinder::MatchResult &Result) { diag(StrFormatCall->getBeginLoc(), "use '%0' instead of %1") << ReplacementFormatFunction << OldFunction->getIdentifier(); Diag << FixItHint::CreateReplacement( - CharSourceRange::getTokenRange(StrFormatCall->getSourceRange()), + CharSourceRange::getTokenRange(StrFormatCall->getExprLoc(), + StrFormatCall->getEndLoc()), ReplacementFormatFunction); Converter.applyFixes(Diag, *Result.SourceManager); diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp index ff990feadc0c1d..16f2f4b3e7d1af 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp @@ -100,8 +100,7 @@ void UseStdPrintCheck::registerMatchers(MatchFinder *Finder) { unusedReturnValue( callExpr(argumentCountAtLeast(1), hasArgument(0, stringLiteral(isOrdinary())), - callee(functionDecl(unless(cxxMethodDecl()), - matchers::matchesAnyListedName( + callee(functionDecl(matchers::matchesAnyListedName( PrintfLikeFunctions)) .bind("func_decl"))) .bind("printf")), @@ -112,8 +111,7 @@ void UseStdPrintCheck::registerMatchers(MatchFinder *Finder) { unusedReturnValue( callExpr(argumentCountAtLeast(2), hasArgument(1, stringLiteral(isOrdinary())), - callee(functionDecl(unless(cxxMethodDecl()), - matchers::matchesAnyListedName( + callee(functionDecl(matchers::matchesAnyListedName( FprintfLikeFunctions)) .bind("func_decl"))) .bind("fprintf")), @@ -152,7 +150,7 @@ void UseStdPrintCheck::check(const MatchFinder::MatchResult &Result) { << ReplacementFunction << OldFunction->getIdentifier(); Diag << FixItHint::CreateReplacement( - CharSourceRange::getTokenRange(PrintfCall->getBeginLoc(), + CharSourceRange::getTokenRange(PrintfCall->getExprLoc(), PrintfCall->getEndLoc()), ReplacementFunction); Converter.applyFixes(Diag, *Result.SourceManager); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index b72d109b3d3938..d542ec4e3e1721 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -104,6 +104,12 @@ New check aliases Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ +- Improved :doc:`modernize-use-std-print + <clang-tidy/checks/modernize/use-std-format>` and + :doc:`modernize-use-std-print + <clang-tidy/checks/modernize/use-std-print>` checks to support replacing + member function calls too. + - Improved :doc:`readability-redundant-smartptr-get <clang-tidy/checks/readability/redundant-smartptr-get>` check to remove `->`, when reduntant `get()` is removed. diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-format.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-format.rst index a1599f0fc58fe6..1ec753ef090de1 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-format.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-format.rst @@ -64,8 +64,9 @@ Options A semicolon-separated list of (fully qualified) function names to replace, with the requirement that the first parameter contains the printf-style format string and the arguments to be formatted follow - immediately afterwards. The default value for this option is - `absl::StrFormat`. + immediately afterwards. Qualified member function names are supported, + but the replacement function name must be unqualified. The default value + for this option is `absl::StrFormat`. .. option:: ReplacementFormatFunction diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-print.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-print.rst index 79648a1104bca2..59bb722e2c24fc 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-print.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-print.rst @@ -121,9 +121,10 @@ Options A semicolon-separated list of (fully qualified) function names to replace, with the requirement that the first parameter contains the printf-style format string and the arguments to be formatted follow - immediately afterwards. If neither this option nor - `FprintfLikeFunctions` are set then the default value for this option - is `printf; absl::PrintF`, otherwise it is empty. + immediately afterwards. Qualified member function names are supported, + but the replacement function name must be unqualified. If neither this + option nor `FprintfLikeFunctions` are set then the default value for + this option is `printf; absl::PrintF`, otherwise it is empty. .. option:: FprintfLikeFunctions @@ -131,9 +132,11 @@ Options A semicolon-separated list of (fully qualified) function names to replace, with the requirement that the first parameter is retained, the second parameter contains the printf-style format string and the - arguments to be formatted follow immediately afterwards. If neither this - option nor `PrintfLikeFunctions` are set then the default value for - this option is `fprintf; absl::FPrintF`, otherwise it is empty. + arguments to be formatted follow immediately afterwards. Qualified + member function names are supported, but the replacement function name + must be unqualified. If neither this option nor `PrintfLikeFunctions` + are set then the default value for this option is `fprintf; + absl::FPrintF`, otherwise it is empty. .. option:: ReplacementPrintFunction diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-member.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-member.cpp new file mode 100644 index 00000000000000..b4e1720e0a106d --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-member.cpp @@ -0,0 +1,44 @@ +// RUN: %check_clang_tidy %s modernize-use-std-format %t -- \ +// RUN: -config="{CheckOptions: \ +// RUN: { \ +// RUN: modernize-use-std-format.StrFormatLikeFunctions: 'MyClass::StrFormat', \ +// RUN: modernize-use-std-format.ReplacementFormatFunction: 'format', \ +// RUN: } \ +// RUN: }" \ +// RUN: -- -isystem %clang_tidy_headers + +#include <cstdio> +#include <string.h> +#include <string> + +struct MyClass +{ + template <typename S, typename... Args> + std::string StrFormat(const S &format, const Args&... args); +}; + +std::string StrFormat_simple(MyClass &myclass, MyClass *pmyclass) { + std::string s; + + s += myclass.StrFormat("MyClass::StrFormat dot %d", 42); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: use 'format' instead of 'StrFormat' [modernize-use-std-format] + // CHECK-FIXES: s += myclass.format("MyClass::StrFormat dot {}", 42); + + s += pmyclass->StrFormat("MyClass::StrFormat pointer %d", 43); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: use 'format' instead of 'StrFormat' [modernize-use-std-format] + // CHECK-FIXES: s += pmyclass->format("MyClass::StrFormat pointer {}", 43); + + s += (*pmyclass).StrFormat("MyClass::StrFormat deref pointer %d", 44); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: use 'format' instead of 'StrFormat' [modernize-use-std-format] + // CHECK-FIXES: s += (*pmyclass).format("MyClass::StrFormat deref pointer {}", 44); + + return s; +} + +struct MyDerivedClass : public MyClass {}; + +std::string StrFormat_derived(MyDerivedClass &derived) { + return derived.StrFormat("MyDerivedClass::StrFormat dot %d", 42); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: use 'format' instead of 'StrFormat' [modernize-use-std-format] + // CHECK-FIXES: return derived.format("MyDerivedClass::StrFormat dot {}", 42); +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-member.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-member.cpp new file mode 100644 index 00000000000000..1d1ac4b369bbaa --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-member.cpp @@ -0,0 +1,70 @@ +// RUN: %check_clang_tidy %s modernize-use-std-print %t -- \ +// RUN: -config="{CheckOptions: \ +// RUN: { \ +// RUN: modernize-use-std-print.PrintfLikeFunctions: 'MyClass::printf', \ +// RUN: modernize-use-std-print.FprintfLikeFunctions: 'MyClass::fprintf', \ +// RUN: modernize-use-std-print.ReplacementPrintFunction: 'print', \ +// RUN: modernize-use-std-print.ReplacementPrintlnFunction: 'println', \ +// RUN: } \ +// RUN: }" \ +// RUN: -- -isystem %clang_tidy_headers + +#include <cstdio> +#include <string.h> + +struct MyStruct {}; + +struct MyClass +{ + template <typename... Args> + void printf(const char *fmt, Args &&...); + template <typename... Args> + int fprintf(MyStruct *param1, const char *fmt, Args &&...); +}; + +void printf_simple(MyClass &myclass, MyClass *pmyclass) { + myclass.printf("printf dot %d", 42); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'print' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES: myclass.print("printf dot {}", 42); + + pmyclass->printf("printf pointer %d", 43); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'print' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES: pmyclass->print("printf pointer {}", 43); + + (*pmyclass).printf("printf deref pointer %d", 44); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'print' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES: (*pmyclass).print("printf deref pointer {}", 44); +} + +void printf_newline(MyClass &myclass, MyClass *pmyclass) { + myclass.printf("printf dot newline %c\n", 'A'); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'println' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES: myclass.println("printf dot newline {}", 'A'); + + pmyclass->printf("printf pointer newline %c\n", 'B'); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'println' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES: pmyclass->println("printf pointer newline {}", 'B'); +} + +void fprintf_simple(MyStruct *mystruct, MyClass &myclass, MyClass *pmyclass) { + myclass.fprintf(mystruct, "fprintf dot %d", 142); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'print' instead of 'fprintf' [modernize-use-std-print] + // CHECK-FIXES: myclass.print(mystruct, "fprintf dot {}", 142); + + pmyclass->fprintf(mystruct, "fprintf pointer %d", 143); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'print' instead of 'fprintf' [modernize-use-std-print] + // CHECK-FIXES: pmyclass->print(mystruct, "fprintf pointer {}", 143); + + (*pmyclass).fprintf(mystruct, "fprintf deref pointer %d", 144); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'print' instead of 'fprintf' [modernize-use-std-print] + // CHECK-FIXES: (*pmyclass).print(mystruct, "fprintf deref pointer {}", 144); +} + +struct MyDerivedClass : public MyClass {}; + +void printf_derived(MyDerivedClass &derived) +{ + derived.printf("printf on derived class %d", 42); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'print' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES: derived.print("printf on derived class {}", 42); +} >From e2a9499e4abe0a276e24696bee515b0dbd0a6fb1 Mon Sep 17 00:00:00 2001 From: Mike Crowe <m...@mcrowe.com> Date: Sat, 17 Aug 2024 20:26:09 +0100 Subject: [PATCH 2/2] fix release notes as PiotrZSL suggested --- clang-tools-extra/docs/ReleaseNotes.rst | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index d542ec4e3e1721..6e281f0f296bc6 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -104,10 +104,12 @@ New check aliases Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ +- Improved :doc:`modernize-use-std-format + <clang-tidy/checks/modernize/use-std-format>` check to support replacing + member function calls too. + - Improved :doc:`modernize-use-std-print - <clang-tidy/checks/modernize/use-std-format>` and - :doc:`modernize-use-std-print - <clang-tidy/checks/modernize/use-std-print>` checks to support replacing + <clang-tidy/checks/modernize/use-std-print>` check to support replacing member function calls too. - Improved :doc:`readability-redundant-smartptr-get _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits