https://github.com/mikecrowe updated https://github.com/llvm/llvm-project/pull/90397
>From 0d6ede5d59cc70d803bfe2c7997737c1be358c75 Mon Sep 17 00:00:00 2001 From: Mike Crowe <m...@mcrowe.com> Date: Sun, 28 Apr 2024 12:41:46 +0100 Subject: [PATCH 1/8] [clang-tidy] Add modernize-use-std-format check Add a new clang-tidy check that converts absl::StrFormat (and similar functions) to std::format (and similar functions.) Split the configuration of FormatStringConverter out to a separate Configuration class so that we don't risk confusion by passing two boolean configuration parameters into the constructor. Add AllowTrailingNewlineRemoval option since we never want to remove trailing newlines in this check. Differential Revision: https://reviews.llvm.org/D154287 --- .../clang-tidy/modernize/CMakeLists.txt | 1 + .../modernize/ModernizeTidyModule.cpp | 2 + .../modernize/UseStdFormatCheck.cpp | 108 ++++++++++++++++++ .../clang-tidy/modernize/UseStdFormatCheck.h | 50 ++++++++ .../clang-tidy/modernize/UseStdPrintCheck.cpp | 5 +- .../utils/FormatStringConverter.cpp | 10 +- .../clang-tidy/utils/FormatStringConverter.h | 9 +- clang-tools-extra/docs/ReleaseNotes.rst | 9 ++ .../docs/clang-tidy/checks/list.rst | 1 + .../checks/modernize/use-std-format.rst | 84 ++++++++++++++ .../modernize/use-std-format-custom.cpp | 76 ++++++++++++ .../checkers/modernize/use-std-format-fmt.cpp | 37 ++++++ .../checkers/modernize/use-std-format.cpp | 97 ++++++++++++++++ 13 files changed, 483 insertions(+), 6 deletions(-) create mode 100644 clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-format.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-fmt.cpp create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index 8005d6e91c060c..576805c4c7f181 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -41,6 +41,7 @@ add_clang_library(clangTidyModernizeModule UseNullptrCheck.cpp UseOverrideCheck.cpp UseStartsEndsWithCheck.cpp + UseStdFormatCheck.cpp UseStdNumbersCheck.cpp UseStdPrintCheck.cpp UseTrailingReturnTypeCheck.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index 776558433c5baa..b9c7a2dc383e88 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -42,6 +42,7 @@ #include "UseNullptrCheck.h" #include "UseOverrideCheck.h" #include "UseStartsEndsWithCheck.h" +#include "UseStdFormatCheck.h" #include "UseStdNumbersCheck.h" #include "UseStdPrintCheck.h" #include "UseTrailingReturnTypeCheck.h" @@ -76,6 +77,7 @@ class ModernizeModule : public ClangTidyModule { "modernize-use-designated-initializers"); CheckFactories.registerCheck<UseStartsEndsWithCheck>( "modernize-use-starts-ends-with"); + CheckFactories.registerCheck<UseStdFormatCheck>("modernize-use-std-format"); CheckFactories.registerCheck<UseStdNumbersCheck>( "modernize-use-std-numbers"); CheckFactories.registerCheck<UseStdPrintCheck>("modernize-use-std-print"); diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp new file mode 100644 index 00000000000000..d22ebe857cf415 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp @@ -0,0 +1,108 @@ +//===--- UseStdFormatCheck.cpp - clang-tidy -------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "UseStdFormatCheck.h" +#include "../utils/FormatStringConverter.h" +#include "../utils/Matchers.h" +#include "../utils/OptionsUtils.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" +#include "clang/Tooling/FixIt.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::modernize { + +namespace { +AST_MATCHER(StringLiteral, isOrdinary) { return Node.isOrdinary(); } +} // namespace + +UseStdFormatCheck::UseStdFormatCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + StrictMode(Options.getLocalOrGlobal("StrictMode", false)), + StrFormatLikeFunctions(utils::options::parseStringList( + Options.get("StrFormatLikeFunctions", ""))), + ReplacementFormatFunction( + Options.get("ReplacementFormatFunction", "std::format")), + IncludeInserter(Options.getLocalOrGlobal("IncludeStyle", + utils::IncludeSorter::IS_LLVM), + areDiagsSelfContained()), + MaybeHeaderToInclude(Options.get("FormatHeader")) { + if (StrFormatLikeFunctions.empty()) + StrFormatLikeFunctions.push_back("absl::StrFormat"); + + if (!MaybeHeaderToInclude && ReplacementFormatFunction == "std::format") + MaybeHeaderToInclude = "<format>"; +} + +void UseStdFormatCheck::registerPPCallbacks(const SourceManager &SM, + Preprocessor *PP, + Preprocessor *ModuleExpanderPP) { + IncludeInserter.registerPreprocessor(PP); +} + +void UseStdFormatCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + callExpr(argumentCountAtLeast(1), + hasArgument(0, stringLiteral(isOrdinary())), + callee(functionDecl(unless(cxxMethodDecl()), + matchers::matchesAnyListedName( + StrFormatLikeFunctions)) + .bind("func_decl"))) + .bind("strformat"), + this); +} + +void UseStdFormatCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + using utils::options::serializeStringList; + Options.store(Opts, "StrictMode", StrictMode); + Options.store(Opts, "StrFormatLikeFunctions", + serializeStringList(StrFormatLikeFunctions)); + Options.store(Opts, "ReplacementFormatFunction", ReplacementFormatFunction); + Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle()); + if (MaybeHeaderToInclude) + Options.store(Opts, "FormatHeader", *MaybeHeaderToInclude); +} + +void UseStdFormatCheck::check(const MatchFinder::MatchResult &Result) { + const unsigned FormatArgOffset = 0; + const auto *OldFunction = Result.Nodes.getNodeAs<FunctionDecl>("func_decl"); + const auto *StrFormat = Result.Nodes.getNodeAs<CallExpr>("strformat"); + + utils::FormatStringConverter::Configuration ConverterConfig; + ConverterConfig.StrictMode = StrictMode; + utils::FormatStringConverter Converter(Result.Context, StrFormat, + FormatArgOffset, ConverterConfig, + getLangOpts()); + const Expr *StrFormatCall = StrFormat->getCallee(); + if (!Converter.canApply()) { + DiagnosticBuilder Diag = diag(StrFormat->getBeginLoc(), + "unable to use '%0' instead of %1 because %2") + << ReplacementFormatFunction + << OldFunction->getIdentifier() + << Converter.conversionNotPossibleReason(); + return; + } + + DiagnosticBuilder Diag = + diag(StrFormatCall->getBeginLoc(), "use '%0' instead of %1") + << ReplacementFormatFunction << OldFunction->getIdentifier(); + Diag << FixItHint::CreateReplacement( + CharSourceRange::getTokenRange(StrFormatCall->getBeginLoc(), + StrFormatCall->getEndLoc()), + ReplacementFormatFunction); + Converter.applyFixes(Diag, *Result.SourceManager); + + if (MaybeHeaderToInclude) + Diag << IncludeInserter.createIncludeInsertion( + Result.Context->getSourceManager().getFileID( + StrFormatCall->getBeginLoc()), + *MaybeHeaderToInclude); +} + +} // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h new file mode 100644 index 00000000000000..d2aeb1bd2802c2 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h @@ -0,0 +1,50 @@ +//===--- UseStdFormatCheck.h - clang-tidy -----------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESTDFORMATCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESTDFORMATCHECK_H + +#include "../ClangTidyCheck.h" + +#include "../ClangTidyCheck.h" +#include "../utils/IncludeInserter.h" + +namespace clang::tidy::modernize { + +/// Convert calls to absl::StrFormat-like functions to std::format. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-std-format.html +class UseStdFormatCheck : public ClangTidyCheck { +public: + UseStdFormatCheck(StringRef Name, ClangTidyContext *Context); + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + if (ReplacementFormatFunction == "std::format") + return LangOpts.CPlusPlus20; + return LangOpts.CPlusPlus; + } + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + std::optional<TraversalKind> getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } + +private: + bool StrictMode; + std::vector<StringRef> StrFormatLikeFunctions; + StringRef ReplacementFormatFunction; + utils::IncludeInserter IncludeInserter; + std::optional<StringRef> MaybeHeaderToInclude; +}; + +} // namespace clang::tidy::modernize + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESTDFORMATCHECK_H diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp index 660996aba7b70d..a4d5fdb852d559 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp @@ -129,8 +129,11 @@ void UseStdPrintCheck::check(const MatchFinder::MatchResult &Result) { FormatArgOffset = 1; } + utils::FormatStringConverter::Configuration ConverterConfig; + ConverterConfig.StrictMode = StrictMode; + ConverterConfig.AllowTrailingNewlineRemoval = true; utils::FormatStringConverter Converter( - Result.Context, Printf, FormatArgOffset, StrictMode, getLangOpts()); + Result.Context, Printf, FormatArgOffset, ConverterConfig, getLangOpts()); const Expr *PrintfCall = Printf->getCallee(); const StringRef ReplacementFunction = Converter.usePrintNewlineFunction() ? ReplacementPrintlnFunction diff --git a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp index ad10f745b6acfb..301300eafb5581 100644 --- a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp +++ b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp @@ -198,10 +198,11 @@ static bool castMismatchedIntegerTypes(const CallExpr *Call, bool StrictMode) { FormatStringConverter::FormatStringConverter(ASTContext *ContextIn, const CallExpr *Call, unsigned FormatArgOffset, - bool StrictMode, + const Configuration ConfigIn, const LangOptions &LO) - : Context(ContextIn), - CastMismatchedIntegerTypes(castMismatchedIntegerTypes(Call, StrictMode)), + : Context(ContextIn), Config(ConfigIn), + CastMismatchedIntegerTypes( + castMismatchedIntegerTypes(Call, ConfigIn.StrictMode)), Args(Call->getArgs()), NumArgs(Call->getNumArgs()), ArgsOffset(FormatArgOffset + 1), LangOpts(LO) { assert(ArgsOffset <= NumArgs); @@ -627,7 +628,8 @@ void FormatStringConverter::finalizeFormatText() { // It's clearer to convert printf("Hello\r\n"); to std::print("Hello\r\n") // than to std::println("Hello\r"); - if (StringRef(StandardFormatString).ends_with("\\n") && + if (Config.AllowTrailingNewlineRemoval && + StringRef(StandardFormatString).ends_with("\\n") && !StringRef(StandardFormatString).ends_with("\\\\n") && !StringRef(StandardFormatString).ends_with("\\r\\n")) { UsePrintNewlineFunction = true; diff --git a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.h b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.h index 1949870f62ed68..1109a0b602262f 100644 --- a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.h +++ b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.h @@ -32,8 +32,14 @@ class FormatStringConverter public: using ConversionSpecifier = clang::analyze_format_string::ConversionSpecifier; using PrintfSpecifier = analyze_printf::PrintfSpecifier; + + struct Configuration { + bool StrictMode = false; + bool AllowTrailingNewlineRemoval = false; + }; + FormatStringConverter(ASTContext *Context, const CallExpr *Call, - unsigned FormatArgOffset, bool StrictMode, + unsigned FormatArgOffset, Configuration Config, const LangOptions &LO); bool canApply() const { return ConversionNotPossibleReason.empty(); } @@ -45,6 +51,7 @@ class FormatStringConverter private: ASTContext *Context; + const Configuration Config; const bool CastMismatchedIntegerTypes; const Expr *const *Args; const unsigned NumArgs; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 3038d2b125f20d..c166801dfd6a60 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -145,6 +145,15 @@ New checks Finds initializer lists for aggregate types that could be written as designated initializers instead. +- New :doc:`modernize-use-std-format + <clang-tidy/checks/modernize/use-std-format>` check. + + Converts calls to ``absl::StrFormat``, or other functions via + configuration options, to C++20's ``std::format``, or another function + via a configuration option, modifying the format string appropriately and + removing now-unnecessary calls to ``std::string::c_str()`` and + ``std::string::data()``. + - New :doc:`readability-enum-initial-value <clang-tidy/checks/readability/enum-initial-value>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 49747ff896ba5c..50cba5b1d21153 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -300,6 +300,7 @@ Clang-Tidy Checks :doc:`modernize-use-nullptr <modernize/use-nullptr>`, "Yes" :doc:`modernize-use-override <modernize/use-override>`, "Yes" :doc:`modernize-use-starts-ends-with <modernize/use-starts-ends-with>`, "Yes" + :doc:`modernize-use-std-format <modernize/use-std-format>`, "Yes" :doc:`modernize-use-std-numbers <modernize/use-std-numbers>`, "Yes" :doc:`modernize-use-std-print <modernize/use-std-print>`, "Yes" :doc:`modernize-use-trailing-return-type <modernize/use-trailing-return-type>`, "Yes" 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 new file mode 100644 index 00000000000000..60dc8ea5e3c46e --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-std-format.rst @@ -0,0 +1,84 @@ +.. title:: clang-tidy - modernize-use-std-format + +modernize-use-std-format +======================== + +Converts calls to ``absl::StrFormat``, or other functions via +configuration options, to C++20's ``std::format``, or another function +via a configuration option, modifying the format string appropriately and +removing now-unnecessary calls to ``std::string::c_str()`` and +``std::string::data()``. + +In other words, it turns lines like: + +.. code-block:: c++ + + return absl::StrFormat("The %s is %3d\n", description.c_str(), value); + +into: + +.. code-block:: c++ + + return std::format("The {} is {:3}", description, value); + +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. + +Options +------- + +.. option:: StrictMode + + When `true`, the check will add casts when converting from variadic + functions and printing signed or unsigned integer types (including + fixed-width integer types from ``<cstdint>``, ``ptrdiff_t``, ``size_t`` + and ``ssize_t``) as the opposite signedness to ensure that the output + would matches that of a simple wrapper for ``std::sprintf`` that + accepted a C-style variable argument list. For example, with + `StrictMode` enabled: + + .. code-block:: c++ + + extern std::string strprintf(const char *format, ...); + int i = -42; + unsigned int u = 0xffffffff; + return strprintf("%d %u\n", i, u); + + would be converted to: + + .. code-block:: c++ + + return std::format("{} {}\n", static_cast<unsigned int>(i), static_cast<int>(u)); + + to ensure that the output will continue to be the unsigned representation + of -42 and the signed representation of 0xffffffff (often 4294967254 + and -1 respectively.) When `false` (which is the default), these casts + will not be added which may cause a change in the output. Note that this + option makes no difference for the default value of + `StrFormatLikeFunctions` since ``absl::StrFormat`` takes a function + parameter pack and is not a variadic function. + +.. option:: StrFormatLikeFunctions + + A semicolon-separated list of (fully qualified) extra 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`. + +.. option:: ReplacementFormatFunction + + The function that will be used to replace the function set by the + `StrFormatLikeFunctions` option rather than the default + `std::format`. It is expected that the function provides an interface + that is compatible with ``std::format``. A suitable candidate would be + `fmt::format`. + +.. option:: FormatHeader + + The header that must be included for the declaration of + `ReplacementFormatFunction` so that a ``#include`` directive can be added if + required. If `ReplacementFormatFunction` is `std::format` then this option will + default to ``<format>``, otherwise this option will default to nothing + and no ``#include`` directive will be added. diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp new file mode 100644 index 00000000000000..bcec9da5cf03a9 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-custom.cpp @@ -0,0 +1,76 @@ +// RUN: %check_clang_tidy -check-suffixes=,STRICT \ +// RUN: -std=c++20 %s modernize-use-std-format %t -- \ +// RUN: -config="{CheckOptions: \ +// RUN: [ \ +// RUN: { \ +// RUN: key: StrictMode, value: true \ +// RUN: }, \ +// RUN: { \ +// RUN: key: modernize-use-std-format.StrFormatLikeFunctions, \ +// RUN: value: 'unqualified_strprintf;::strprintf; mynamespace::strprintf2' \ +// RUN: }, \ +// RUN: { \ +// RUN: key: modernize-use-std-format.ReplacementFormatFunction, \ +// RUN: value: 'fmt::format' \ +// RUN: }, \ +// RUN: { \ +// RUN: key: modernize-use-std-format.FormatHeader, \ +// RUN: value: '<fmt/core.h>' \ +// RUN: } \ +// RUN: ] \ +// RUN: }" \ +// RUN: -- -isystem %clang_tidy_headers +// RUN: %check_clang_tidy -check-suffixes=,NOTSTRICT \ +// RUN: -std=c++20 %s modernize-use-std-format %t -- \ +// RUN: -config="{CheckOptions: \ +// RUN: [ \ +// RUN: { \ +// RUN: key: modernize-use-std-format.StrFormatLikeFunctions, \ +// RUN: value: '::strprintf; mynamespace::strprintf2' \ +// RUN: }, \ +// RUN: { \ +// RUN: key: modernize-use-std-format.ReplacementFormatFunction, \ +// RUN: value: 'fmt::format' \ +// RUN: }, \ +// RUN: { \ +// RUN: key: modernize-use-std-format.FormatHeader, \ +// RUN: value: '<fmt/core.h>' \ +// RUN: } \ +// RUN: ] \ +// RUN: }" \ +// RUN: -- -isystem %clang_tidy_headers + +#include <cstdio> +#include <string> +// CHECK-FIXES: #include <fmt/core.h> + +std::string strprintf(const char *, ...); + +namespace mynamespace { + std::string strprintf2(const char *, ...); +} + +std::string strprintf_test(const std::string &name, double value) { + return strprintf("'%s'='%f'\n", name.c_str(), value); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: use 'fmt::format' instead of 'strprintf' [modernize-use-std-format] + // CHECK-FIXES: return fmt::format("'{}'='{:f}'\n", name, value); + + return mynamespace::strprintf2("'%s'='%f'\n", name.c_str(), value); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: use 'fmt::format' instead of 'strprintf2' [modernize-use-std-format] + // CHECK-FIXES: return fmt::format("'{}'='{:f}'\n", name, value); +} + +std::string StrFormat_strict_conversion() { + const unsigned char uc = 'A'; + return strprintf("Integer %hhd from unsigned char\n", uc); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: use 'fmt::format' instead of 'strprintf' [modernize-use-std-format] + // CHECK-FIXES-NOTSTRICT: return fmt::format("Integer {} from unsigned char\n", uc); + // CHECK-FIXES-STRICT: return fmt::format("Integer {} from unsigned char\n", static_cast<signed char>(uc)); +} + +// Ensure that MatchesAnyListedNameMatcher::NameMatcher::match() can cope with a +// NamedDecl that has no name when we're trying to match unqualified_strprintf. +std::string A(const std::string &in) +{ + return "_" + in; +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-fmt.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-fmt.cpp new file mode 100644 index 00000000000000..5ed894a6c74b73 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format-fmt.cpp @@ -0,0 +1,37 @@ +// RUN: %check_clang_tidy %s modernize-use-std-format %t -- \ +// RUN: -config="{CheckOptions: \ +// RUN: [ \ +// RUN: { \ +// RUN: key: StrictMode, value: true \ +// RUN: }, \ +// RUN: { \ +// RUN: key: modernize-use-std-format.StrFormatLikeFunctions, \ +// RUN: value: 'fmt::sprintf' \ +// RUN: }, \ +// RUN: { \ +// RUN: key: modernize-use-std-format.ReplacementFormatFunction, \ +// RUN: value: 'fmt::format' \ +// RUN: }, \ +// RUN: { \ +// RUN: key: modernize-use-std-format.FormatHeader, \ +// RUN: value: '<fmt/core.h>' \ +// RUN: } \ +// RUN: ] \ +// RUN: }" \ +// RUN: -- -isystem %clang_tidy_headers + +// CHECK-FIXES: #include <fmt/core.h> +#include <string> + +namespace fmt +{ +// Use const char * for the format since the real type is hard to mock up. +template <typename... Args> +std::string sprintf(const char *format, const Args&... args); +} // namespace fmt + +std::string fmt_sprintf_simple() { + return fmt::sprintf("Hello %s %d", "world", 42); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: use 'fmt::format' instead of 'sprintf' [modernize-use-std-format] + // CHECK-FIXES: fmt::format("Hello {} {}", "world", 42); +} 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 new file mode 100644 index 00000000000000..8ea4bd875eaed1 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-format.cpp @@ -0,0 +1,97 @@ +// RUN: %check_clang_tidy \ +// RUN: -std=c++20 %s modernize-use-std-format %t -- \ +// RUN: -config="{CheckOptions: [{key: StrictMode, value: true}]}" \ +// RUN: -- -isystem %clang_tidy_headers +// RUN: %check_clang_tidy \ +// RUN: -std=c++20 %s modernize-use-std-format %t -- \ +// RUN: -config="{CheckOptions: [{key: StrictMode, value: false}]}" \ +// RUN: -- -isystem %clang_tidy_headers +#include <string> +// CHECK-FIXES: #include <format> + +namespace absl +{ +// Use const char * for the format since the real type is hard to mock up. +template <typename... Args> +std::string StrFormat(const char *format, const Args&... args); +} // namespace absl + +template <typename T> +struct iterator { + T *operator->(); + T &operator*(); +}; + +std::string StrFormat_simple() { + return absl::StrFormat("Hello"); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format] + // CHECK-FIXES: return std::format("Hello"); +} + +std::string StrFormat_complex(const char *name, double value) { + return absl::StrFormat("'%s'='%f'", name, value); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format] + // CHECK-FIXES: return std::format("'{}'='{:f}'", name, value); +} + +std::string StrFormat_integer_conversions() { + return absl::StrFormat("int:%d int:%d char:%c char:%c", 65, 'A', 66, 'B'); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format] + // CHECK-FIXES: return std::format("int:{} int:{:d} char:{:c} char:{}", 65, 'A', 66, 'B'); +} + +// FormatConverter is capable of removing newlines from the end of the format +// string. Ensure that isn't incorrectly happening for std::format. +std::string StrFormat_no_newline_removal() { + return absl::StrFormat("a line\n"); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format] + // CHECK-FIXES: return std::format("a line\n"); +} + +// FormatConverter is capable of removing newlines from the end of the format +// string. Ensure that isn't incorrectly happening for std::format. +std::string StrFormat_cstr_removal(const std::string &s1, const std::string *s2) { + return absl::StrFormat("%s %s %s %s", s1.c_str(), s1.data(), s2->c_str(), s2->data()); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format] + // CHECK-FIXES: return std::format("{} {} {} {}", s1, s1, *s2, *s2); +} + +std::string StrFormat_strict_conversion() { + const unsigned char uc = 'A'; + return absl::StrFormat("Integer %hhd from unsigned char\n", uc); + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format] + // CHECK-FIXES: return std::format("Integer {} from unsigned char\n", uc); +} + +std::string StrFormat_field_width_and_precision() { + auto s1 = absl::StrFormat("width only:%*d width and precision:%*.*f precision only:%.*f", 3, 42, 4, 2, 3.14159265358979323846, 5, 2.718); + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format] + // CHECK-FIXES: std::format("width only:{:{}} width and precision:{:{}.{}f} precision only:{:.{}f}", 42, 3, 3.14159265358979323846, 4, 2, 2.718, 5); + + auto s2 = absl::StrFormat("width and precision positional:%1$*2$.*3$f after", 3.14159265358979323846, 4, 2); + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format] + // CHECK-FIXES: std::format("width and precision positional:{0:{1}.{2}f} after", 3.14159265358979323846, 4, 2); + + const int width = 10, precision = 3; + const unsigned int ui1 = 42, ui2 = 43, ui3 = 44; + auto s3 = absl::StrFormat("casts width only:%*d width and precision:%*.*d precision only:%.*d\n", 3, ui1, 4, 2, ui2, 5, ui3); + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format] + // CHECK-FIXES-NOTSTRICT: std::format("casts width only:{:{}} width and precision:{:{}.{}} precision only:{:.{}}", ui1, 3, ui2, 4, 2, ui3, 5); + // CHECK-FIXES-STRICT: std::format("casts width only:{:{}} width and precision:{:{}.{}} precision only:{:.{}}", static_cast<int>(ui1), 3, static_cast<int>(ui2), 4, 2, static_cast<int>(ui3), 5); + + auto s4 = absl::StrFormat("c_str removal width only:%*s width and precision:%*.*s precision only:%.*s", 3, s1.c_str(), 4, 2, s2.c_str(), 5, s3.c_str()); + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format] + // CHECK-FIXES: std::format("c_str removal width only:{:>{}} width and precision:{:>{}.{}} precision only:{:.{}}", s1, 3, s2, 4, 2, s3, 5); + + const std::string *ps1 = &s1, *ps2 = &s2, *ps3 = &s3; + auto s5 = absl::StrFormat("c_str() removal pointer width only:%-*s width and precision:%-*.*s precision only:%-.*s", 3, ps1->c_str(), 4, 2, ps2->c_str(), 5, ps3->c_str()); + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format] + // CHECK-FIXES: std::format("c_str() removal pointer width only:{:{}} width and precision:{:{}.{}} precision only:{:.{}}", *ps1, 3, *ps2, 4, 2, *ps3, 5); + + iterator<std::string> is1, is2, is3; + auto s6 = absl::StrFormat("c_str() removal iterator width only:%-*s width and precision:%-*.*s precision only:%-.*s", 3, is1->c_str(), 4, 2, is2->c_str(), 5, is3->c_str()); + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use 'std::format' instead of 'StrFormat' [modernize-use-std-format] + // CHECK-FIXES: std::format("c_str() removal iterator width only:{:{}} width and precision:{:{}.{}} precision only:{:.{}}", *is1, 3, *is2, 4, 2, *is3, 5); + + return s1 + s2 + s3 + s4 + s5 + s6; +} >From 00ca5cf6ab08b28c2a4c57f573b5732676f7fadf Mon Sep 17 00:00:00 2001 From: Mike Crowe <m...@mcrowe.com> Date: Sun, 28 Apr 2024 15:44:50 +0100 Subject: [PATCH 2/8] fixup! [clang-tidy] Add modernize-use-std-format check Remove duplicate inclusion of ClangTidyCheck.h. --- clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h index d2aeb1bd2802c2..dd17f609005c35 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h @@ -9,8 +9,6 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESTDFORMATCHECK_H #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESTDFORMATCHECK_H -#include "../ClangTidyCheck.h" - #include "../ClangTidyCheck.h" #include "../utils/IncludeInserter.h" >From 366308bc3540f66c219186a0e3a947b649e55291 Mon Sep 17 00:00:00 2001 From: Mike Crowe <m...@mcrowe.com> Date: Sun, 28 Apr 2024 16:51:57 +0100 Subject: [PATCH 3/8] fixup! [clang-tidy] Add modernize-use-std-format check Use getSourceRange rather than getBeginLoc()/getEndLoc(). --- clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp index d22ebe857cf415..a589e40056d457 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp @@ -93,8 +93,7 @@ void UseStdFormatCheck::check(const MatchFinder::MatchResult &Result) { diag(StrFormatCall->getBeginLoc(), "use '%0' instead of %1") << ReplacementFormatFunction << OldFunction->getIdentifier(); Diag << FixItHint::CreateReplacement( - CharSourceRange::getTokenRange(StrFormatCall->getBeginLoc(), - StrFormatCall->getEndLoc()), + CharSourceRange::getTokenRange(StrFormatCall->getSourceRange()), ReplacementFormatFunction); Converter.applyFixes(Diag, *Result.SourceManager); >From 97058482857dc707978ec958392b638f5fc28920 Mon Sep 17 00:00:00 2001 From: Mike Crowe <m...@mcrowe.com> Date: Sun, 28 Apr 2024 16:54:06 +0100 Subject: [PATCH 4/8] fixup! [clang-tidy] Add modernize-use-std-format check Synchronise description of check in UseStdFormatCheck.h with the more-detailed description in the release notes. --- clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h index dd17f609005c35..b59a4708c6e4bc 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.h @@ -14,7 +14,10 @@ namespace clang::tidy::modernize { -/// Convert calls to absl::StrFormat-like functions to std::format. +/// Converts calls to absl::StrFormat, or other functions via configuration +/// options, to C++20's std::format, or another function via a configuration +/// option, modifying the format string appropriately and removing +/// now-unnecessary calls to std::string::c_str() and std::string::data(). /// /// For the user-facing documentation see: /// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-std-format.html >From 5c9fc69abf607c9da3964a21a946fb2a1ba01987 Mon Sep 17 00:00:00 2001 From: Mike Crowe <m...@mcrowe.com> Date: Sun, 28 Apr 2024 16:56:36 +0100 Subject: [PATCH 5/8] fixup! [clang-tidy] Add modernize-use-std-format check .) -> ). --- .../docs/clang-tidy/checks/modernize/use-std-format.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 60dc8ea5e3c46e..5457b0a7c98aa3 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 @@ -53,7 +53,7 @@ Options to ensure that the output will continue to be the unsigned representation of -42 and the signed representation of 0xffffffff (often 4294967254 - and -1 respectively.) When `false` (which is the default), these casts + and -1 respectively). When `false` (which is the default), these casts will not be added which may cause a change in the output. Note that this option makes no difference for the default value of `StrFormatLikeFunctions` since ``absl::StrFormat`` takes a function >From 56313daffe207d2651096dd27c3b784f1fb1ccfc Mon Sep 17 00:00:00 2001 From: Mike Crowe <m...@mcrowe.com> Date: Sun, 28 Apr 2024 17:17:41 +0100 Subject: [PATCH 6/8] fixup! [clang-tidy] Add modernize-use-std-format check Add some tests that include macro expansions. --- .../checkers/modernize/use-std-format.cpp | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) 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 8ea4bd875eaed1..57947275176c61 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 @@ -95,3 +95,26 @@ std::string StrFormat_field_width_and_precision() { return s1 + s2 + s3 + s4 + s5 + s6; } + +std::string StrFormat_macros() { + // The function call is replaced even though it comes from a macro. +#define FORMAT absl::StrFormat + auto s1 = FORMAT("Hello %d", 42); + // 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 <inttypes.h> 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 +#define PRECISION 4 + 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); +} >From f31f47eb6871ad1afe0e422c0e6bb2567c1c0b23 Mon Sep 17 00:00:00 2001 From: Mike Crowe <m...@mcrowe.com> Date: Sun, 28 Apr 2024 17:27:38 +0100 Subject: [PATCH 7/8] fixup! [clang-tidy] Add modernize-use-std-format check Avoid calling StringRef constructor multiple times. --- .../clang-tidy/utils/FormatStringConverter.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp index 301300eafb5581..845e71c5003b80 100644 --- a/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp +++ b/clang-tools-extra/clang-tidy/utils/FormatStringConverter.cpp @@ -628,10 +628,12 @@ void FormatStringConverter::finalizeFormatText() { // It's clearer to convert printf("Hello\r\n"); to std::print("Hello\r\n") // than to std::println("Hello\r"); + // Use StringRef until C++20 std::string::ends_with() is available. + const auto StandardFormatStringRef = StringRef(StandardFormatString); if (Config.AllowTrailingNewlineRemoval && - StringRef(StandardFormatString).ends_with("\\n") && - !StringRef(StandardFormatString).ends_with("\\\\n") && - !StringRef(StandardFormatString).ends_with("\\r\\n")) { + StandardFormatStringRef.ends_with("\\n") && + !StandardFormatStringRef.ends_with("\\\\n") && + !StandardFormatStringRef.ends_with("\\r\\n")) { UsePrintNewlineFunction = true; FormatStringNeededRewriting = true; StandardFormatString.erase(StandardFormatString.end() - 2, >From 0f4b068d35c290d5a85c2dff4fe86b78b8d30689 Mon Sep 17 00:00:00 2001 From: Mike Crowe <m...@mcrowe.com> Date: Sun, 28 Apr 2024 17:31:11 +0100 Subject: [PATCH 8/8] fixup! [clang-tidy] Add modernize-use-std-format check Run clang-format --- clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp index a589e40056d457..fd38cbb12506f5 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseStdFormatCheck.cpp @@ -93,7 +93,7 @@ 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->getSourceRange()), ReplacementFormatFunction); Converter.applyFixes(Diag, *Result.SourceManager); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
