From 2dd0902d5f79a2b18f53649169bd011ccfbfb46b Mon Sep 17 00:00:00 2001 From: Mike Crowe Date: Wed, 12 Jun 2024 21:06:26 +0100 Subject: [PATCH 1/2] [clang-tidy] Only expand macros in modernize-use-std-format/print Expanding all macros in the printf/absl::StrFormat format string before conversion could easily break code if those macros are expended to change their definition between builds. It's important for this check to expand the PRI macros though, so let's ensure that the presence of any other macros in the format string causes the check to emit a warning and not perform any conversion. --- .../modernize/UseStdFormatCheck.cpp | 7 +- .../clang-tidy/modernize/UseStdFormatCheck.h | 1 + .../clang-tidy/modernize/UseStdPrintCheck.cpp | 4 +- .../clang-tidy/modernize/UseStdPrintCheck.h | 1 + .../utils/FormatStringConverter.cpp | 65 +++++++++++++-- .../clang-tidy/utils/FormatStringConverter.h | 7 +- clang-tools-extra/docs/ReleaseNotes.rst | 10 +++ .../checks/modernize/use-std-format.rst | 3 +- .../checks/modernize/use-std-print.rst | 22 +++--- .../checkers/Inputs/Headers/inttypes.h | 26 +++--- .../checkers/modernize/use-std-format.cpp | 79 ++++++++++++++++--- .../checkers/modernize/use-std-print.cpp | 73 ++++++++++++++++- 12 files changed, 257 insertions(+), 41 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp index cdb34aef1b0e61..f97b844c564961 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp @@ -44,6 +44,7 @@ void UseStdFormatCheck::registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { IncludeInserter.registerPreprocessor(PP); + this->PP = PP; } void UseStdFormatCheck::registerMatchers(MatchFinder *Finder) { @@ -75,9 +76,9 @@ void UseStdFormatCheck::check(const MatchFinder::MatchResult &Result) { utils::FormatStringConverter::Configuration ConverterConfig; ConverterConfig.StrictMode = StrictMode; - utils::FormatStringConverter Converter(Result.Context, StrFormat, - FormatArgOffset, ConverterConfig, - getLangOpts()); + utils::FormatStringConverter Converter( + Result.Context, StrFormat, FormatArgOffset, ConverterConfig, + getLangOpts(), *Result.SourceManager, *PP); const Expr *StrFormatCall = StrFormat->getCallee(); if (!Converter.canApply()) { diag(StrFormat->getBeginLoc(), diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h index b59a4708c6e4bc..9ac2240212ebf6 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h @@ -44,6 +44,7 @@ class UseStdFormatCheck : public ClangTidyCheck { StringRef ReplacementFormatFunction; utils::IncludeInserter IncludeInserter; std::optional MaybeHeaderToInclude; + Preprocessor *PP = nullptr; }; } // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp index 16f2f4b3e7d1af..9161c0e702a28c 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp @@ -68,6 +68,7 @@ void UseStdPrintCheck::registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { IncludeInserter.registerPreprocessor(PP); + this->PP = PP; } static clang::ast_matchers::StatementMatcher @@ -131,7 +132,8 @@ void UseStdPrintCheck::check(const MatchFinder::MatchResult &Result) { ConverterConfig.StrictMode = StrictMode; ConverterConfig.AllowTrailingNewlineRemoval = true; utils::FormatStringConverter Converter( - Result.Context, Printf, FormatArgOffset, ConverterConfig, getLangOpts()); + Result.Context, Printf, FormatArgOffset, ConverterConfig, getLangOpts(), + *Result.SourceManager, *PP); const Expr *PrintfCall = Printf->getCallee(); const StringRef ReplacementFunction = Converter.usePrintNewlineFunction() ? ReplacementPrintlnFunction diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.h b/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.h index 7a06cf38b4264f..995c740389e73b 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.h @@ -36,6 +36,7 @@ class UseStdPrintCheck : public ClangTidyCheck { } private: + Preprocessor *PP; bool StrictMode; std::vector PrintfLikeFunctions; std::vector FprintfLikeFunctions; diff --git a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp index 33f3ea47df1e38..7f4ccca84faa58 100644 --- a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp +++ b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp @@ -18,6 +18,7 @@ #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Basic/LangOptions.h" #include "clang/Lex/Lexer.h" +#include "clang/Lex/Preprocessor.h" #include "clang/Tooling/FixIt.h" #include "llvm/ADT/StringExtras.h" #include "llvm/Support/Debug.h" @@ -195,11 +196,10 @@ static bool castMismatchedIntegerTypes(const CallExpr *Call, bool StrictMode) { return false; } -FormatStringConverter::FormatStringConverter(ASTContext *ContextIn, - const CallExpr *Call, - unsigned FormatArgOffset, - const Configuration ConfigIn, - const LangOptions &LO) +FormatStringConverter::FormatStringConverter( + ASTContext *ContextIn, const CallExpr *Call, unsigned FormatArgOffset, + const Configuration ConfigIn, const LangOptions &LO, SourceManager &SM, + Preprocessor &PP) : Context(ContextIn), Config(ConfigIn), CastMismatchedIntegerTypes( castMismatchedIntegerTypes(Call, ConfigIn.StrictMode)), @@ -208,11 +208,22 @@ FormatStringConverter::FormatStringConverter(ASTContext *ContextIn, assert(ArgsOffset <= NumArgs); FormatExpr = llvm::dyn_cast( Args[FormatArgOffset]->IgnoreImplicitAsWritten()); + if (!FormatExpr || !FormatExpr->isOrdinary()) { // Function must have a narrow string literal as its first argument. conversionNotPossible("first argument is not a narrow string literal"); return; } + + if (const std::optional MaybeMacroName = + formatStringContainsUnreplaceableMacro(Call, FormatExpr, SM, PP); + MaybeMacroName) { + conversionNotPossible( + ("format string contains unreplaceable macro '" + *MaybeMacroName + "'") + .str()); + return; + } + PrintfFormatString = FormatExpr->getString(); // Assume that the output will be approximately the same size as the input, @@ -230,6 +241,50 @@ FormatStringConverter::FormatStringConverter(ASTContext *ContextIn, finalizeFormatText(); } +std::optional +FormatStringConverter::formatStringContainsUnreplaceableMacro( + const CallExpr *Call, const StringLiteral *FormatExpr, SourceManager &SM, + Preprocessor &PP) { + // If a macro invocation surrounds the entire call then we don't want that to + // inhibit conversion. The whole format string will appear to come from that + // macro, as will the function call. + std::optional MaybeSurroundingMacroName; + if (SourceLocation BeginCallLoc = Call->getBeginLoc(); + BeginCallLoc.isMacroID()) + MaybeSurroundingMacroName = + Lexer::getImmediateMacroName(BeginCallLoc, SM, PP.getLangOpts()); + + for (auto I = FormatExpr->tokloc_begin(), E = FormatExpr->tokloc_end(); + I != E; ++I) { + const SourceLocation &TokenLoc = *I; + if (TokenLoc.isMacroID()) { + const StringRef MacroName = + Lexer::getImmediateMacroName(TokenLoc, SM, PP.getLangOpts()); + + if (MaybeSurroundingMacroName != MacroName) { + // glibc uses __PRI64_PREFIX and __PRIPTR_PREFIX to define the prefixes + // for types that change size so we must look for multiple prefixes. + if (!MacroName.starts_with("PRI") && !MacroName.starts_with("__PRI")) + return MacroName; + + const SourceLocation TokenSpellingLoc = SM.getSpellingLoc(TokenLoc); + const OptionalFileEntryRef MaybeFileEntry = + SM.getFileEntryRefForID(SM.getFileID(TokenSpellingLoc)); + if (!MaybeFileEntry) + return MacroName; + + HeaderSearch &HS = PP.getHeaderSearchInfo(); + // Check if the file is a system header + if (!isSystem(HS.getFileDirFlavor(*MaybeFileEntry)) || + llvm::sys::path::filename(MaybeFileEntry->getName()) != + "inttypes.h") + return MacroName; + } + } + } + return std::nullopt; +} + void FormatStringConverter::emitAlignment(const PrintfSpecifier &FS, std::string &FormatSpec) { ConversionSpecifier::Kind ArgKind = FS.getConversionSpecifier().getKind(); diff --git a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.h b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.h index 1109a0b602262f..15d1f597fe440c 100644 --- a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.h +++ b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.h @@ -40,7 +40,8 @@ class FormatStringConverter FormatStringConverter(ASTContext *Context, const CallExpr *Call, unsigned FormatArgOffset, Configuration Config, - const LangOptions &LO); + const LangOptions &LO, SourceManager &SM, + Preprocessor &PP); bool canApply() const { return ConversionNotPossibleReason.empty(); } const std::string &conversionNotPossibleReason() const { @@ -110,6 +111,10 @@ class FormatStringConverter void appendFormatText(StringRef Text); void finalizeFormatText(); + static std::optional + formatStringContainsUnreplaceableMacro(const CallExpr *CallExpr, + const StringLiteral *FormatExpr, + SourceManager &SM, Preprocessor &PP); bool conversionNotPossible(std::string Reason) { ConversionNotPossibleReason = std::move(Reason); return false; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index b4792d749a86c3..ca99ceedb450b9 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -194,10 +194,20 @@ Changes in existing checks ` check to support replacing member function calls too. +- Improved :doc:`modernize-use-std-format + ` check to only expand macros + starting with ``PRI`` and ``__PRI`` from ```` in the format + string. + - Improved :doc:`modernize-use-std-print ` check to support replacing member function calls too. +- Improved :doc:`modernize-use-std-print + ` check to only expand macros + starting with ``PRI`` and ``__PRI`` from ```` in the format + string. + - Improved :doc:`performance-avoid-endl ` check to use ``std::endl`` as placeholder when lexer cannot get source text. 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 b88fde5162e28c..cfa11d3cac8bfe 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 @@ -23,7 +23,8 @@ into: The check uses the same format-string-conversion algorithm as `modernize-use-std-print <../modernize/use-std-print.html>`_ and its -shortcomings are described in the documentation for that check. +shortcomings and behaviour in combination with macros are described in the +documentation for that check. Options ------- 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 e70402ad8b3341..0cf51e3961a05c 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 @@ -24,8 +24,15 @@ into: std::println(stderr, "The {} is {:3}", description, value); If the `ReplacementPrintFunction` or `ReplacementPrintlnFunction` options -are left, or assigned to their default values then this check is only -enabled with `-std=c++23` or later. +are left at or set to their default values then this check is only enabled +with `-std=c++23` or later. + +Macros starting with ``PRI`` and ``__PRI`` from `` are +expanded, escaping is handled and adjacent strings are concatenated to form +a single ``StringLiteral`` before the format string is converted. Use of +any other macros in the format string will cause a warning message to be +emitted and no conversion will be performed. The converted format string +will always be a single string literal. The check doesn't do a bad job, but it's not perfect. In particular: @@ -34,13 +41,10 @@ The check doesn't do a bad job, but it's not perfect. In particular: possible. - At the point that the check runs, the AST contains a single - ``StringLiteral`` for the format string and any macro expansion, token - pasting, adjacent string literal concatenation and escaping has been - handled. Although it's possible for the check to automatically put the - escapes back, they may not be exactly as they were written (e.g. - ``"\x0a"`` will become ``"\n"`` and ``"ab" "cd"`` will become - ``"abcd"``.) This is helpful since it means that the ``PRIx`` macros from - ```` are removed correctly. + ``StringLiteral`` for the format string where escapes have been expanded. + The check tries to reconstruct escape sequences, they may not be the same + as they were written (e.g. ``"\x41\x0a"`` will become ``"A\n"`` and + ``"ab" "cd"`` will become ``"abcd"``.) - It supports field widths, precision, positional arguments, leading zeros, leading ``+``, alignment and alternative forms. diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/inttypes.h b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/inttypes.h index 9dc7ae39b3a3f0..74437f405931b2 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/inttypes.h +++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/inttypes.h @@ -21,40 +21,46 @@ typedef __UINT32_TYPE__ uint32_t; typedef __UINT16_TYPE__ uint16_t; typedef __UINT8_TYPE__ uint8_t; -#define PRIdMAX "lld" -#define PRId64 "lld" +#if __WORDSIZE == 64 +# define __PRI64_PREFIX "l" +#else +# define __PRI64_PREFIX "ll" +#endif + +#define PRIdMAX __PRI64_PREFIX "d" +#define PRId64 __PRI64_PREFIX "d" #define PRId32 "d" #define PRId16 "hd" #define PRId8 "hhd" -#define PRIiMAX "lli" -#define PRIi64 "lli" +#define PRIiMAX __PRI64_PREFIX "i" +#define PRIi64 __PRI64_PREFIX "i" #define PRIi32 "i" #define PRIi16 "hi" #define PRIi8 "hhi" -#define PRIiFAST64 "lli" +#define PRIiFAST64 __PRI64_PREFIX "i" #define PRIiFAST32 "i" #define PRIiFAST16 "hi" #define PRIiFAST8 "hhi" -#define PRIiLEAST64 "lli" +#define PRIiLEAST64 __PRI64_PREFIX "i" #define PRIiLEAST32 "i" #define PRIiLEAST16 "hi" #define PRIiLEAST8 "hhi" -#define PRIuMAX "llu" -#define PRIu64 "llu" +#define PRIuMAX __PRI64_PREFIX "u" +#define PRIu64 __PRI64_PREFIX "u" #define PRIu32 "u" #define PRIu16 "hu" #define PRIu8 "hhu" -#define PRIuFAST64 "llu" +#define PRIuFAST64 __PRI64_PREFIX "u" #define PRIuFAST32 "u" #define PRIuFAST16 "hu" #define PRIuFAST8 "hhu" -#define PRIuLEAST64 "llu" +#define PRIuLEAST64 __PRI64_PREFIX "u" #define PRIuLEAST32 "u" #define PRIuLEAST16 "hu" #define PRIuLEAST8 "hhu" diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format.cpp index 800a95062e8f1a..42fb3382e4a936 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format.cpp @@ -1,13 +1,18 @@ // RUN: %check_clang_tidy \ // RUN: -std=c++20 %s modernize-use-std-format %t -- \ // RUN: -config="{CheckOptions: {StrictMode: true}}" \ -// RUN: -- -isystem %clang_tidy_headers +// RUN: -- -isystem %clang_tidy_headers \ +// RUN: -DPRI_CMDLINE_MACRO="\"s\"" \ +// RUN: -D__PRI_CMDLINE_MACRO="\"s\"" // RUN: %check_clang_tidy \ // RUN: -std=c++20 %s modernize-use-std-format %t -- \ // RUN: -config="{CheckOptions: {StrictMode: false}}" \ -// RUN: -- -isystem %clang_tidy_headers +// RUN: -- -isystem %clang_tidy_headers \ +// RUN: -DPRI_CMDLINE_MACRO="\"s\"" \ +// RUN: -D__PRI_CMDLINE_MACRO="\"s\"" #include // CHECK-FIXES: #include +#include namespace absl { @@ -102,13 +107,6 @@ std::string StrFormat_macros() { // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format] // CHECK-FIXES: std::format("Hello {}", 42); - // The format string is replaced even though it comes from a macro, this - // behaviour is required so that that macros are replaced. -#define FORMAT_STRING "Hello %s" - auto s2 = absl::StrFormat(FORMAT_STRING, 42); - // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format] - // CHECK-FIXES: std::format("Hello {}", 42); - // Arguments that are macros aren't replaced with their value, even if they are rearranged. #define VALUE 3.14159265358979323846 #define WIDTH 10 @@ -116,4 +114,67 @@ std::string StrFormat_macros() { auto s3 = absl::StrFormat("Hello %*.*f", WIDTH, PRECISION, VALUE); // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format] // CHECK-FIXES: std::format("Hello {:{}.{}f}", VALUE, WIDTH, PRECISION); + + const uint64_t u64 = 42; + const uint32_t u32 = 32; + std::string s; + + auto s4 = absl::StrFormat("Replaceable macro at end %" PRIu64, u64); + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format] + // CHECK-FIXES: std::format("Replaceable macro at end {}", u64); + + auto s5 = absl::StrFormat("Replaceable macros in middle %" PRIu64 " %" PRIu32 "\n", u64, u32); + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format] + // CHECK-FIXES: std::format("Replaceable macros in middle {} {}\n", u64, u32); + +// These need PRI and __PRI prefixes so that the check get as far as looking for +// where the macro comes from. +#define PRI_FMT_MACRO "s" +#define __PRI_FMT_MACRO "s" + + auto s6 = absl::StrFormat("Unreplaceable macro at end %" PRI_FMT_MACRO, s.c_str()); + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: unable to use 'std::format' instead of 'StrFormat' because format string contains unreplaceable macro 'PRI_FMT_MACRO' [modernize-use-std-format] + + auto s7 = absl::StrFormat(__PRI_FMT_MACRO " Unreplaceable macro at beginning %s", s); + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: unable to use 'std::format' instead of 'StrFormat' because format string contains unreplaceable macro '__PRI_FMT_MACRO' [modernize-use-std-format] + + auto s8 = absl::StrFormat("Unreplacemable macro %" PRI_FMT_MACRO " in the middle", s); + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: unable to use 'std::format' instead of 'StrFormat' because format string contains unreplaceable macro 'PRI_FMT_MACRO' [modernize-use-std-format] + + auto s9 = absl::StrFormat("First macro is replaceable %" PRIu64 " but second one is not %" __PRI_FMT_MACRO, u64, s); + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: unable to use 'std::format' instead of 'StrFormat' because format string contains unreplaceable macro '__PRI_FMT_MACRO' [modernize-use-std-format] + + // Needs a PRI prefix so that we get as far as looking for where the macro comes from + auto s10 = absl::StrFormat(" macro from command line %" PRI_CMDLINE_MACRO, s); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: unable to use 'std::format' instead of 'StrFormat' because format string contains unreplaceable macro 'PRI_CMDLINE_MACRO' [modernize-use-std-format] + + // Needs a __PRI prefix so that we get as far as looking for where the macro comes from + auto s11 = absl::StrFormat(" macro from command line %" __PRI_CMDLINE_MACRO, s); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: unable to use 'std::format' instead of 'StrFormat' because format string contains unreplaceable macro '__PRI_CMDLINE_MACRO' [modernize-use-std-format] + + // We ought to be able to fix this since the macro surrounds the whole call + // and therefore can't change the format string independently. This is + // required to be able to fix calls inside Catch2 macros for example. +#define SURROUND_ALL(x) x + auto s12 = SURROUND_ALL(absl::StrFormat("Macro surrounding entire invocation %" PRIu64, u64)); + // CHECK-MESSAGES: [[@LINE-1]]:27: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format] + // CHECK-FIXES: auto s12 = SURROUND_ALL(std::format("Macro surrounding entire invocation {}", u64)); + + // But having that surrounding macro shouldn't stop us ignoring an + // unreplaceable macro elsewhere. + auto s13 = SURROUND_ALL(absl::StrFormat("Macro surrounding entire invocation with unreplaceable macro %" PRI_FMT_MACRO, s)); + // CHECK-MESSAGES: [[@LINE-1]]:27: warning: unable to use 'std::format' instead of 'StrFormat' because format string contains unreplaceable macro 'PRI_FMT_MACRO' [modernize-use-std-format] + + // At the moment at least the check will replace occurrences where the + // function name is the result of expanding a macro. +#define SURROUND_FUNCTION_NAME(x) absl:: x + auto s14 = SURROUND_FUNCTION_NAME(StrFormat)("Hello %d", 4442); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format] + // CHECK-FIXES: auto s14 = std::format("Hello {}", 4442); + + // We can't safely fix occurrences where the macro may affect the format + // string differently in different builds. +#define SURROUND_FORMAT(x) "!" x + auto s15 = absl::StrFormat(SURROUND_FORMAT("Hello %d"), 4443); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: unable to use 'std::format' instead of 'StrFormat' because format string contains unreplaceable macro 'SURROUND_FORMAT' [modernize-use-std-format] } diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp index da1a18782c9bed..f11fc408fcb9c8 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print.cpp @@ -1,11 +1,15 @@ // RUN: %check_clang_tidy -check-suffixes=,STRICT \ // RUN: -std=c++23 %s modernize-use-std-print %t -- \ // RUN: -config="{CheckOptions: {StrictMode: true}}" \ -// RUN: -- -isystem %clang_tidy_headers -fexceptions +// RUN: -- -isystem %clang_tidy_headers -fexceptions \ +// RUN: -DPRI_CMDLINE_MACRO="\"s\"" \ +// RUN: -D__PRI_CMDLINE_MACRO="\"s\"" // RUN: %check_clang_tidy -check-suffixes=,NOTSTRICT \ // RUN: -std=c++23 %s modernize-use-std-print %t -- \ // RUN: -config="{CheckOptions: {StrictMode: false}}" \ -// RUN: -- -isystem %clang_tidy_headers -fexceptions +// RUN: -- -isystem %clang_tidy_headers -fexceptions \ +// RUN: -DPRI_CMDLINE_MACRO="\"s\"" \ +// RUN: -D__PRI_CMDLINE_MACRO="\"s\"" #include #include #include @@ -1571,3 +1575,68 @@ void p(S s1, S *s2) // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::print' instead of 'printf' [modernize-use-std-print] // CHECK-FIXES: std::print("Not std::string {} {}", s1.data(), s2->data()); } + +// These need PRI and __PRI prefixes so that the check gets as far as looking +// for where the macro comes from. +#define PRI_FMT_MACRO "s" +#define __PRI_FMT_MACRO "s" + +void macro_expansion(const char *s) +{ + const uint64_t u64 = 42; + const uint32_t u32 = 32; + + printf("Replaceable macro at end %" PRIu64, u64); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::print' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES: std::print("Replaceable macro at end {}", u64); + + printf("Replaceable macros in middle %" PRIu64 " %" PRIu32 "\n", u64, u32); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use 'std::println' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES: std::println("Replaceable macros in middle {} {}", u64, u32); + + printf("Unreplaceable macro at end %" PRI_FMT_MACRO, s); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: unable to use 'std::print' instead of 'printf' because format string contains unreplaceable macro 'PRI_FMT_MACRO' [modernize-use-std-print] + + printf(PRI_FMT_MACRO " Unreplaceable macro at beginning %s", s); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: unable to use 'std::print' instead of 'printf' because format string contains unreplaceable macro 'PRI_FMT_MACRO' [modernize-use-std-print] + + printf("Unreplacemable macro %" __PRI_FMT_MACRO " in the middle", s); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: unable to use 'std::print' instead of 'printf' because format string contains unreplaceable macro '__PRI_FMT_MACRO' [modernize-use-std-print] + + printf("First macro is replaceable %" PRIu64 " but second one is not %" PRI_FMT_MACRO, u64, s); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: unable to use 'std::print' instead of 'printf' because format string contains unreplaceable macro 'PRI_FMT_MACRO' [modernize-use-std-print] + + // Needs a PRI prefix so that we get as far as looking for where the macro comes from + printf(" macro from command line %" PRI_CMDLINE_MACRO, s); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: unable to use 'std::print' instead of 'printf' because format string contains unreplaceable macro 'PRI_CMDLINE_MACRO' [modernize-use-std-print] + + // Needs a __PRI prefix so that we get as far as looking for where the macro comes from + printf(" macro from command line %" __PRI_CMDLINE_MACRO, s); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: unable to use 'std::print' instead of 'printf' because format string contains unreplaceable macro '__PRI_CMDLINE_MACRO' [modernize-use-std-print] + + // We ought to be able to fix this since the macro surrounds the whole call + // and therefore can't change the format string independently. This is + // required to be able to fix calls inside Catch2 macros for example. +#define SURROUND_ALL(x) x + SURROUND_ALL(printf("Macro surrounding entire invocation %" PRIu64, u64)); + // CHECK-MESSAGES: [[@LINE-1]]:16: warning: use 'std::print' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES: SURROUND_ALL(std::print("Macro surrounding entire invocation {}", u64)); + + // But having that surrounding macro shouldn't stop us ignoring an + // unreplaceable macro elsewhere. + SURROUND_ALL(printf("Macro surrounding entire invocation with unreplaceable macro %" PRI_FMT_MACRO, s)); + // CHECK-MESSAGES: [[@LINE-1]]:16: warning: unable to use 'std::print' instead of 'printf' because format string contains unreplaceable macro 'PRI_FMT_MACRO' [modernize-use-std-print] + + // At the moment at least the check will replace occurrences where the + // function name is the result of expanding a macro. +#define SURROUND_FUNCTION_NAME(x) x + SURROUND_FUNCTION_NAME(printf)("Hello %d", 4442); + // CHECK-MESSAGES: [[@LINE-1]]:26: warning: use 'std::print' instead of 'printf' [modernize-use-std-print] + // CHECK-FIXES: std::print("Hello {}", 4442); + + // We can't safely fix occurrences where the macro may affect the format + // string differently in different builds. +#define SURROUND_FORMAT(x) "!" x + printf(SURROUND_FORMAT("Hello %d"), 4443); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: unable to use 'std::print' instead of 'printf' because format string contains unreplaceable macro 'SURROUND_FORMAT' [modernize-use-std-print] +} From 91a8f8d030e5edd8525431fda56998576e5f1454 Mon Sep 17 00:00:00 2001 From: Mike Crowe Date: Sun, 6 Oct 2024 14:25:04 +0100 Subject: [PATCH 2/2] Merge entries in release notes --- clang-tools-extra/docs/ReleaseNotes.rst | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index ca99ceedb450b9..bebc060a8f4f46 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -192,21 +192,13 @@ Changes in existing checks - Improved :doc:`modernize-use-std-format ` check to support replacing - member function calls too. - -- Improved :doc:`modernize-use-std-format - ` check to only expand macros - starting with ``PRI`` and ``__PRI`` from ```` in the format - string. + member function calls too and to only expand macros starting with ``PRI`` + and ``__PRI`` from ```` in the format string. - Improved :doc:`modernize-use-std-print ` check to support replacing - member function calls too. - -- Improved :doc:`modernize-use-std-print - ` check to only expand macros - starting with ``PRI`` and ``__PRI`` from ```` in the format - string. + member function calls too and to only expand macros starting with ``PRI`` + and ``__PRI`` from ```` in the format string. - Improved :doc:`performance-avoid-endl ` check to use ``std::endl`` as