https://github.com/kastiglione updated https://github.com/llvm/llvm-project/pull/195974
>From 4a3911083c1d07236693b6bd40d1e02344a05b6c Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Tue, 5 May 2026 18:34:33 -0700 Subject: [PATCH 01/28] [clang-tidy] Add `llvm-formatv-string` --- .../clang-tidy/llvm/CMakeLists.txt | 1 + .../clang-tidy/llvm/FormatvStringCheck.cpp | 202 ++++++++++++++++++ .../clang-tidy/llvm/FormatvStringCheck.h | 42 ++++ .../clang-tidy/llvm/LLVMTidyModule.cpp | 2 + .../docs/clang-tidy/checks/list.rst | 1 + .../clang-tidy/checks/llvm/formatv-string.rst | 60 ++++++ .../llvm/formatv-string-additional.cpp | 28 +++ .../checkers/llvm/formatv-string.cpp | 58 +++++ 8 files changed, 394 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-additional.cpp create mode 100644 clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp diff --git a/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt b/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt index c81882e0e2024..bec3ba50c81c5 100644 --- a/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt @@ -4,6 +4,7 @@ set(LLVM_LINK_COMPONENTS ) add_clang_library(clangTidyLLVMModule STATIC + FormatvStringCheck.cpp HeaderGuardCheck.cpp IncludeOrderCheck.cpp LLVMTidyModule.cpp diff --git a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp new file mode 100644 index 0000000000000..6bfd66b002b47 --- /dev/null +++ b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp @@ -0,0 +1,202 @@ +//===----------------------------------------------------------------------===// +// +// 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 "FormatvStringCheck.h" +#include "clang/AST/Expr.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallBitVector.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Error.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::llvm_check { + +namespace { + +struct ParseResult { + llvm::SmallVector<unsigned, 4> Indices; + unsigned MaxIndex = 0; +}; + +} // namespace + +static llvm::Expected<ParseResult> parseFormatvString(llvm::StringRef Fmt) { + ParseResult Result; + unsigned NextAutoIndex = 0; + bool HasAutomatic = false; + bool HasExplicit = false; + + while (!Fmt.empty()) { + const size_t OpenBrace = Fmt.find('{'); + if (OpenBrace == llvm::StringRef::npos) + break; + + Fmt = Fmt.drop_front(OpenBrace); + + // Handle escaped braces '{{'. + if (Fmt.size() > 1 && Fmt[1] == '{') { + Fmt = Fmt.drop_front(2); + continue; + } + + // Find the closing '}'. + const size_t CloseBrace = Fmt.find('}'); + if (CloseBrace == llvm::StringRef::npos) + return llvm::createStringError("unterminated brace in format string"); + + // Extract the content between braces. + llvm::StringRef Content = Fmt.substr(1, CloseBrace - 1); + Fmt = Fmt.drop_front(CloseBrace + 1); + + // Parse the replacement field: [index] ["," layout] [":" format] + llvm::StringRef IndexStr = Content; + + // Strip layout and format parts for index parsing. + const size_t CommaPos = Content.find(','); + const size_t ColonPos = Content.find(':'); + if (CommaPos != llvm::StringRef::npos) + IndexStr = Content.substr(0, CommaPos); + else if (ColonPos != llvm::StringRef::npos) + IndexStr = Content.substr(0, ColonPos); + + IndexStr = IndexStr.trim(); + + unsigned Index = 0; + if (IndexStr.empty()) { + Index = NextAutoIndex++; + HasAutomatic = true; + } else { + if (IndexStr.getAsInteger(10, Index)) + return llvm::createStringError("invalid replacement index"); + HasExplicit = true; + } + + Result.Indices.push_back(Index); + Result.MaxIndex = std::max(Result.MaxIndex, Index); + } + + if (HasAutomatic && HasExplicit) + return llvm::createStringError( + "format string mixes automatic and explicit indices"); + + return Result; +} + +FormatvStringCheck::FormatvStringCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + AdditionalFunctions(Options.get("AdditionalFunctions", "")) { + // Always check llvm::formatv (both overloads). + Functions["llvm::formatv"] = 0; + + // Parse "name1:idx1;name2:idx2;..." from AdditionalFunctions. + llvm::StringRef Input(AdditionalFunctions); + while (!Input.empty()) { + auto [Entry, Rest] = Input.split(';'); + Input = Rest; + if (Entry.empty()) + continue; + auto [Name, IdxStr] = Entry.rsplit(':'); + unsigned Idx = 0; + if (Name.empty() || IdxStr.empty() || IdxStr.getAsInteger(10, Idx)) { + configurationDiag("invalid entry '%0' in option AdditionalFunctions, " + "expected 'fully::qualified::name:fmt_arg_index'") + << Entry; + continue; + } + Functions[Name] = Idx; + } +} + +void FormatvStringCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "AdditionalFunctions", AdditionalFunctions); +} + +void FormatvStringCheck::registerMatchers(MatchFinder *Finder) { + // Build a matcher for all configured function names. + std::vector<llvm::StringRef> Names; + Names.reserve(Functions.size()); + llvm::copy(Functions.keys(), std::back_inserter(Names)); + + Finder->addMatcher( + callExpr(callee(functionDecl(hasAnyName(Names)))).bind("call"), this); +} + +void FormatvStringCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Call = Result.Nodes.getNodeAs<CallExpr>("call"); + if (!Call || Call->getNumArgs() == 0) + return; + + const auto *FD = Call->getDirectCallee(); + if (!FD) + return; + + // Look up the format string parameter index for this function. + const std::string QualName = FD->getQualifiedNameAsString(); + assert(Functions.contains(QualName) && + "matched function not in Functions map"); + unsigned FmtArgIdx = Functions.lookup(QualName); + + // For llvm::formatv, also handle the (bool, const char*, ...) overload. + if (QualName == "llvm::formatv" && FD->getNumParams() > 0 && + FD->getParamDecl(0)->getType()->isBooleanType()) + FmtArgIdx = 1; + + if (Call->getNumArgs() <= FmtArgIdx) + return; + + // Extract the format string literal. + const Expr *FmtArg = Call->getArg(FmtArgIdx)->IgnoreParenImpCasts(); + const auto *FmtLiteral = dyn_cast<StringLiteral>(FmtArg); + if (!FmtLiteral) + return; + + llvm::StringRef FmtStr = FmtLiteral->getString(); + const unsigned FirstArgIdx = FmtArgIdx + 1; + const int NumArgs = Call->getNumArgs() - FirstArgIdx; + + auto ParsedOrErr = parseFormatvString(FmtStr); + if (!ParsedOrErr) { + diag(FmtLiteral->getBeginLoc(), "formatv() %0") + << llvm::toString(ParsedOrErr.takeError()); + return; + } + + const ParseResult &Parsed = *ParsedOrErr; + const int NumRequiredArgs = Parsed.Indices.empty() ? 0 : Parsed.MaxIndex + 1; + + if (NumRequiredArgs != NumArgs) { + diag(FmtLiteral->getBeginLoc(), + "formatv() format string requires %0 argument(s), but %1 " + "argument(s) were provided") + << NumRequiredArgs << NumArgs; + return; + } + + // Check for holes in indices. + if (!Parsed.Indices.empty()) { + llvm::SmallBitVector UsedIndices(NumRequiredArgs); + for (unsigned Index : Parsed.Indices) + UsedIndices.set(Index); + + const int UnusedIndex = UsedIndices.find_first_unset(); + if (0 <= UnusedIndex && UnusedIndex < NumRequiredArgs) { + // Point to the unused argument. + const Expr *UnusedArg = Call->getArg(FirstArgIdx + UnusedIndex); + diag(UnusedArg->getBeginLoc(), + "formatv() format string does not use argument at index %0") + << UnusedIndex; + return; + } + } +} + +} // namespace clang::tidy::llvm_check diff --git a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.h b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.h new file mode 100644 index 0000000000000..ef9a759dca248 --- /dev/null +++ b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.h @@ -0,0 +1,42 @@ +//===----------------------------------------------------------------------===// +// +// 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_LLVM_FORMATVSTRINGCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_FORMATVSTRINGCHECK_H + +#include "../ClangTidyCheck.h" +#include "llvm/ADT/StringMap.h" + +namespace clang::tidy::llvm_check { + +/// Validates llvm::formatv format strings against the provided arguments. +/// +/// Checks that: +/// - The number of format indices matches the number of arguments. +/// - Every argument is used by the format string. +/// - Automatic and explicit indices are not mixed. +class FormatvStringCheck : public ClangTidyCheck { +public: + FormatvStringCheck(StringRef Name, ClangTidyContext *Context); + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + // Map from fully-qualified function name to the 0-based index of the format + // string parameter. + llvm::StringMap<unsigned> Functions; + const std::string AdditionalFunctions; +}; + +} // namespace clang::tidy::llvm_check + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_FORMATVSTRINGCHECK_H diff --git a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp index 104fcf63712f7..918af88c979e0 100644 --- a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp @@ -11,6 +11,7 @@ #include "../readability/ElseAfterReturnCheck.h" #include "../readability/NamespaceCommentCheck.h" #include "../readability/QualifiedAutoCheck.h" +#include "FormatvStringCheck.h" #include "HeaderGuardCheck.h" #include "IncludeOrderCheck.h" #include "PreferIsaOrDynCastInConditionalsCheck.h" @@ -32,6 +33,7 @@ class LLVMModule : public ClangTidyModule { void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.registerCheck<readability::ElseAfterReturnCheck>( "llvm-else-after-return"); + CheckFactories.registerCheck<FormatvStringCheck>("llvm-formatv-string"); CheckFactories.registerCheck<LLVMHeaderGuardCheck>("llvm-header-guard"); CheckFactories.registerCheck<IncludeOrderCheck>("llvm-include-order"); CheckFactories.registerCheck<readability::NamespaceCommentCheck>( diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 053ce6f0779d9..59dabbd0041e3 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -244,6 +244,7 @@ Clang-Tidy Checks :doc:`google-upgrade-googletest-case <google/upgrade-googletest-case>`, "Yes" :doc:`hicpp-multiway-paths-covered <hicpp/multiway-paths-covered>`, :doc:`linuxkernel-must-check-errs <linuxkernel/must-check-errs>`, + :doc:`llvm-formatv-string <llvm/formatv-string>`, :doc:`llvm-header-guard <llvm/header-guard>`, :doc:`llvm-include-order <llvm/include-order>`, "Yes" :doc:`llvm-namespace-comment <llvm/namespace-comment>`, diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst b/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst new file mode 100644 index 0000000000000..945522dc0d2a1 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst @@ -0,0 +1,60 @@ +.. title:: clang-tidy - llvm-formatv-string + +llvm-formatv-string +=================== + +Validates ``llvm::formatv`` format strings against the arguments provided, +similar to how the compiler validates ``printf`` format strings. + +This check diagnoses the following issues: + +- The number of replacement indices in the format string does not match the + number of arguments provided. +- A format string does not use an argument at a given index. +- Automatic and explicit indices are mixed (e.g. ``{} {1}``). + +.. code-block:: c++ + + // Warning: requires 2 arguments, but 1 provided. + llvm::formatv("{0} {1}", x); + + // Warning: mixes automatic and explicit indices. + llvm::formatv("{} {1}", x, y); + + // Warning: format string does not use argument at index 1. + llvm::formatv("{0} {2}", x, y, z); + + // OK. + llvm::formatv("{0} {1}", x, y); + llvm::formatv("{} {}", x, y); + llvm::formatv("{0} {0}", x); + +The check only operates on calls where the format string is a string literal. +Dynamic format strings are not diagnosed. + +Options +------- + +.. option:: AdditionalFunctions + + A semicolon-separated list of additional functions to check, beyond + ``llvm::formatv``. Each entry has the form ``name:index``, where ``name`` + is the fully qualified function name and ``index`` is the 0-based parameter + position of the format string. + + For example, to check ``mylib::log(Level, const char *Fmt, ...)`` where + the format string is the second parameter (index 1): + + .. code-block:: yaml + + CheckOptions: + llvm-formatv-string.AdditionalFunctions: "mylib::log:1" + + Multiple entries are separated by semicolons: + + .. code-block:: yaml + + CheckOptions: + llvm-formatv-string.AdditionalFunctions: "mylib::log:1;lldb_private::Log::Format:2" + + Default: empty. diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-additional.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-additional.cpp new file mode 100644 index 0000000000000..47c881e489ebf --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-additional.cpp @@ -0,0 +1,28 @@ +// RUN: %check_clang_tidy %s llvm-formatv-string %t -- \ +// RUN: -config='{CheckOptions: {llvm-formatv-string.AdditionalFunctions: "mylib::log:1"}}' + +namespace llvm { + +template <typename... Ts> +void formatv(const char *Fmt, Ts &&...Vals) {} + +} // namespace llvm + +namespace mylib { + +enum Level { Info, Error }; + +template <typename... Ts> +void log(Level L, const char *Fmt, Ts &&...Vals) {} + +} // namespace mylib + +void correct() { + mylib::log(mylib::Info, "{0} {1}", 1, 2); + mylib::log(mylib::Error, "{0}", 42); +} + +void wrong_count() { + mylib::log(mylib::Info, "{0} {1}", 1); + // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: formatv() format string requires 2 argument(s), but 1 argument(s) were provided +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp new file mode 100644 index 0000000000000..fc268dd45802c --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp @@ -0,0 +1,58 @@ +// RUN: %check_clang_tidy %s llvm-formatv-string %t + +namespace llvm { + +template <typename... Ts> +void formatv(const char *Fmt, Ts &&...Vals) {} + +template <typename... Ts> +void formatv(bool Validate, const char *Fmt, Ts &&...Vals) {} + +} // namespace llvm + +void correct() { + llvm::formatv("{0}", 1); + llvm::formatv("{0} {1}", 1, 2); + llvm::formatv("{0} {0}", 1); + llvm::formatv("{1} {0}", 1, 2); + llvm::formatv("{0,10}", 1); + llvm::formatv("{0,-10}", 1); + llvm::formatv("{0:x}", 1); + llvm::formatv("{0,10:x}", 1); + llvm::formatv("no replacements"); + llvm::formatv("escaped {{ braces }}"); + llvm::formatv("{}", 1); + llvm::formatv("{} {}", 1, 2); + llvm::formatv(false, "{0}", 1); +} + +void too_few_args() { + llvm::formatv("{0} {1}", 1); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: formatv() format string requires 2 argument(s), but 1 argument(s) were provided + + llvm::formatv("{0} {1} {2}", 1, 2); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: formatv() format string requires 3 argument(s), but 2 argument(s) were provided +} + +void too_many_args() { + llvm::formatv("{0}", 1, 2); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: formatv() format string requires 1 argument(s), but 2 argument(s) were provided + + llvm::formatv("no replacements", 1); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: formatv() format string requires 0 argument(s), but 1 argument(s) were provided +} + +void mixed_indices() { + llvm::formatv("{} {1}", 1, 2); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: formatv() format string mixes automatic and explicit indices +} + +void holes_in_indices() { + llvm::formatv("{0} {2}", 1, 2, 3); + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: formatv() format string does not use argument at index 1 +} + +void non_literal_format_string(const char *fmt) { + // No warning for non-literal format strings. + llvm::formatv(fmt, 1, 2); +} >From 492473405818a0b0572a9cb62fc49a20acf59e11 Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Tue, 5 May 2026 18:48:36 -0700 Subject: [PATCH 02/28] Address misc-const-correctness --- clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp index 6bfd66b002b47..cf778d948e370 100644 --- a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp @@ -53,7 +53,7 @@ static llvm::Expected<ParseResult> parseFormatvString(llvm::StringRef Fmt) { return llvm::createStringError("unterminated brace in format string"); // Extract the content between braces. - llvm::StringRef Content = Fmt.substr(1, CloseBrace - 1); + const llvm::StringRef Content = Fmt.substr(1, CloseBrace - 1); Fmt = Fmt.drop_front(CloseBrace + 1); // Parse the replacement field: [index] ["," layout] [":" format] @@ -159,7 +159,7 @@ void FormatvStringCheck::check(const MatchFinder::MatchResult &Result) { if (!FmtLiteral) return; - llvm::StringRef FmtStr = FmtLiteral->getString(); + const llvm::StringRef FmtStr = FmtLiteral->getString(); const unsigned FirstArgIdx = FmtArgIdx + 1; const int NumArgs = Call->getNumArgs() - FirstArgIdx; @@ -184,7 +184,7 @@ void FormatvStringCheck::check(const MatchFinder::MatchResult &Result) { // Check for holes in indices. if (!Parsed.Indices.empty()) { llvm::SmallBitVector UsedIndices(NumRequiredArgs); - for (unsigned Index : Parsed.Indices) + for (const unsigned Index : Parsed.Indices) UsedIndices.set(Index); const int UnusedIndex = UsedIndices.find_first_unset(); >From ffad4371821956c77eea6382c3e01853d7a1c8f8 Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Tue, 5 May 2026 21:17:27 -0700 Subject: [PATCH 03/28] Apply `const` suggestion from @EugeneZelenko Co-authored-by: EugeneZelenko <[email protected]> --- clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp index cf778d948e370..daf011e262610 100644 --- a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp @@ -100,7 +100,7 @@ FormatvStringCheck::FormatvStringCheck(StringRef Name, // Parse "name1:idx1;name2:idx2;..." from AdditionalFunctions. llvm::StringRef Input(AdditionalFunctions); while (!Input.empty()) { - auto [Entry, Rest] = Input.split(';'); + const auto [Entry, Rest] = Input.split(';'); Input = Rest; if (Entry.empty()) continue; >From aeb0312c3da142d55bd653e4081997665f3299cc Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Tue, 5 May 2026 21:17:44 -0700 Subject: [PATCH 04/28] Apply `const` suggestion from @EugeneZelenko Co-authored-by: EugeneZelenko <[email protected]> --- clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp index daf011e262610..767915aa21531 100644 --- a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp @@ -104,7 +104,7 @@ FormatvStringCheck::FormatvStringCheck(StringRef Name, Input = Rest; if (Entry.empty()) continue; - auto [Name, IdxStr] = Entry.rsplit(':'); + const auto [Name, IdxStr] = Entry.rsplit(':'); unsigned Idx = 0; if (Name.empty() || IdxStr.empty() || IdxStr.getAsInteger(10, Idx)) { configurationDiag("invalid entry '%0' in option AdditionalFunctions, " >From 2d58e21cc9115e07976e1317dce5f76a6c62f684 Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Tue, 5 May 2026 21:18:57 -0700 Subject: [PATCH 05/28] Apply documentation suggestion from @EugeneZelenko Co-authored-by: EugeneZelenko <[email protected]> --- .../docs/clang-tidy/checks/llvm/formatv-string.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst b/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst index 945522dc0d2a1..a3d60d1b11555 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst @@ -38,8 +38,8 @@ Options .. option:: AdditionalFunctions A semicolon-separated list of additional functions to check, beyond - ``llvm::formatv``. Each entry has the form ``name:index``, where ``name`` - is the fully qualified function name and ``index`` is the 0-based parameter + ``llvm::formatv``. Each entry has the form `name:index`, where `name` is the + fully qualified function name and `index` is the zero-based parameter position of the format string. For example, to check ``mylib::log(Level, const char *Fmt, ...)`` where >From 49604bb11600ed2fcbc77f39d63db9d0f4d3af13 Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Tue, 5 May 2026 21:19:19 -0700 Subject: [PATCH 06/28] Apply documentation suggestion from @EugeneZelenko Co-authored-by: EugeneZelenko <[email protected]> --- .../docs/clang-tidy/checks/llvm/formatv-string.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst b/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst index a3d60d1b11555..f9b6f408d2a7b 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst @@ -42,8 +42,8 @@ Options fully qualified function name and `index` is the zero-based parameter position of the format string. - For example, to check ``mylib::log(Level, const char *Fmt, ...)`` where - the format string is the second parameter (index 1): + For example, to check `mylib::log(Level, const char *Fmt, ...)` where the + format string is the second parameter (index 1): .. code-block:: yaml >From 9567c433535c4d962ba11d08668256999cbdf68f Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Tue, 5 May 2026 21:19:37 -0700 Subject: [PATCH 07/28] Apply documentation suggestion from @EugeneZelenko Co-authored-by: EugeneZelenko <[email protected]> --- .../docs/clang-tidy/checks/llvm/formatv-string.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst b/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst index f9b6f408d2a7b..3fc1ea7c21a99 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst @@ -57,4 +57,4 @@ Options CheckOptions: llvm-formatv-string.AdditionalFunctions: "mylib::log:1;lldb_private::Log::Format:2" - Default: empty. + Default is empty string. >From 96fe06275c8d8c3566a476c3182d09fcc0657d19 Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Tue, 5 May 2026 21:20:45 -0700 Subject: [PATCH 08/28] Apply option documentation suggestion from @EugeneZelenko Co-authored-by: EugeneZelenko <[email protected]> --- .../docs/clang-tidy/checks/llvm/formatv-string.rst | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst b/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst index 3fc1ea7c21a99..233c960d8eb39 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst @@ -45,16 +45,4 @@ Options For example, to check `mylib::log(Level, const char *Fmt, ...)` where the format string is the second parameter (index 1): - .. code-block:: yaml - - CheckOptions: - llvm-formatv-string.AdditionalFunctions: "mylib::log:1" - - Multiple entries are separated by semicolons: - - .. code-block:: yaml - - CheckOptions: - llvm-formatv-string.AdditionalFunctions: "mylib::log:1;lldb_private::Log::Format:2" - Default is empty string. >From af6d7cb9d6a60b6c4309f63a62897f3e94adc155 Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Wed, 6 May 2026 17:24:13 -0700 Subject: [PATCH 09/28] Address feedback and make further improvements --- .../clang-tidy/llvm/FormatvStringCheck.cpp | 52 +++++++++---------- .../clang-tidy/llvm/FormatvStringCheck.h | 7 +++ .../llvm/formatv-string-additional.cpp | 2 +- .../checkers/llvm/formatv-string.cpp | 10 ++-- 4 files changed, 38 insertions(+), 33 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp index 767915aa21531..f64e452b92838 100644 --- a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp @@ -12,7 +12,6 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallBitVector.h" #include "llvm/ADT/SmallVector.h" -#include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" using namespace clang::ast_matchers; @@ -104,15 +103,15 @@ FormatvStringCheck::FormatvStringCheck(StringRef Name, Input = Rest; if (Entry.empty()) continue; - const auto [Name, IdxStr] = Entry.rsplit(':'); - unsigned Idx = 0; - if (Name.empty() || IdxStr.empty() || IdxStr.getAsInteger(10, Idx)) { + const auto [Name, IndexStr] = Entry.rsplit(':'); + unsigned Index = 0; + if (Name.empty() || IndexStr.empty() || IndexStr.getAsInteger(10, Index)) { configurationDiag("invalid entry '%0' in option AdditionalFunctions, " "expected 'fully::qualified::name:fmt_arg_index'") << Entry; continue; } - Functions[Name] = Idx; + Functions[Name] = Index; } } @@ -127,43 +126,43 @@ void FormatvStringCheck::registerMatchers(MatchFinder *Finder) { llvm::copy(Functions.keys(), std::back_inserter(Names)); Finder->addMatcher( - callExpr(callee(functionDecl(hasAnyName(Names)))).bind("call"), this); + callExpr(callee(functionDecl(hasAnyName(Names))), argumentCountAtLeast(1)) + .bind("call"), + this); } void FormatvStringCheck::check(const MatchFinder::MatchResult &Result) { const auto *Call = Result.Nodes.getNodeAs<CallExpr>("call"); - if (!Call || Call->getNumArgs() == 0) - return; + assert(Call && Call->getNumArgs() > 0); const auto *FD = Call->getDirectCallee(); - if (!FD) - return; + assert(FD); - // Look up the format string parameter index for this function. + // Look up the index of the format string parameter for this function. const std::string QualName = FD->getQualifiedNameAsString(); assert(Functions.contains(QualName) && "matched function not in Functions map"); - unsigned FmtArgIdx = Functions.lookup(QualName); + unsigned FmtStringIndex = Functions.lookup(QualName); // For llvm::formatv, also handle the (bool, const char*, ...) overload. if (QualName == "llvm::formatv" && FD->getNumParams() > 0 && FD->getParamDecl(0)->getType()->isBooleanType()) - FmtArgIdx = 1; + FmtStringIndex = 1; - if (Call->getNumArgs() <= FmtArgIdx) + if (Call->getNumArgs() <= FmtStringIndex) return; // Extract the format string literal. - const Expr *FmtArg = Call->getArg(FmtArgIdx)->IgnoreParenImpCasts(); + const Expr *FmtArg = Call->getArg(FmtStringIndex)->IgnoreParenImpCasts(); const auto *FmtLiteral = dyn_cast<StringLiteral>(FmtArg); if (!FmtLiteral) return; - const llvm::StringRef FmtStr = FmtLiteral->getString(); - const unsigned FirstArgIdx = FmtArgIdx + 1; - const int NumArgs = Call->getNumArgs() - FirstArgIdx; + const llvm::StringRef FmtString = FmtLiteral->getString(); + const unsigned FirstFmtArgIndex = FmtStringIndex + 1; + const int NumFmtArgs = Call->getNumArgs() - FirstFmtArgIndex; - auto ParsedOrErr = parseFormatvString(FmtStr); + auto ParsedOrErr = parseFormatvString(FmtString); if (!ParsedOrErr) { diag(FmtLiteral->getBeginLoc(), "formatv() %0") << llvm::toString(ParsedOrErr.takeError()); @@ -173,15 +172,15 @@ void FormatvStringCheck::check(const MatchFinder::MatchResult &Result) { const ParseResult &Parsed = *ParsedOrErr; const int NumRequiredArgs = Parsed.Indices.empty() ? 0 : Parsed.MaxIndex + 1; - if (NumRequiredArgs != NumArgs) { + if (NumRequiredArgs != NumFmtArgs) { diag(FmtLiteral->getBeginLoc(), - "formatv() format string requires %0 argument(s), but %1 " - "argument(s) were provided") - << NumRequiredArgs << NumArgs; + "formatv() format string requires %0 argument%s0, but %1 argument%s1 " + "%plural{1:was|:were}1 provided") + << NumRequiredArgs << NumFmtArgs; return; } - // Check for holes in indices. + // Check for unused arguments. if (!Parsed.Indices.empty()) { llvm::SmallBitVector UsedIndices(NumRequiredArgs); for (const unsigned Index : Parsed.Indices) @@ -190,10 +189,9 @@ void FormatvStringCheck::check(const MatchFinder::MatchResult &Result) { const int UnusedIndex = UsedIndices.find_first_unset(); if (0 <= UnusedIndex && UnusedIndex < NumRequiredArgs) { // Point to the unused argument. - const Expr *UnusedArg = Call->getArg(FirstArgIdx + UnusedIndex); + const Expr *UnusedArg = Call->getArg(FirstFmtArgIndex + UnusedIndex); diag(UnusedArg->getBeginLoc(), - "formatv() format string does not use argument at index %0") - << UnusedIndex; + "formatv() argument unused in format string"); return; } } diff --git a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.h b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.h index ef9a759dca248..aefd02730c12b 100644 --- a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.h +++ b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.h @@ -20,6 +20,9 @@ namespace clang::tidy::llvm_check { /// - The number of format indices matches the number of arguments. /// - Every argument is used by the format string. /// - Automatic and explicit indices are not mixed. +/// +/// For the user-facing documentation see: +/// https://clang.llvm.org/extra/clang-tidy/checks/llvm/formatv-string.html class FormatvStringCheck : public ClangTidyCheck { public: FormatvStringCheck(StringRef Name, ClangTidyContext *Context); @@ -30,6 +33,10 @@ class FormatvStringCheck : public ClangTidyCheck { 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: // Map from fully-qualified function name to the 0-based index of the format // string parameter. diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-additional.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-additional.cpp index 47c881e489ebf..f42603ee3e19f 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-additional.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-additional.cpp @@ -24,5 +24,5 @@ void correct() { void wrong_count() { mylib::log(mylib::Info, "{0} {1}", 1); - // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: formatv() format string requires 2 argument(s), but 1 argument(s) were provided + // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: formatv() format string requires 2 arguments, but 1 argument was provided } diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp index fc268dd45802c..66707e95455bb 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp @@ -28,18 +28,18 @@ void correct() { void too_few_args() { llvm::formatv("{0} {1}", 1); - // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: formatv() format string requires 2 argument(s), but 1 argument(s) were provided + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: formatv() format string requires 2 arguments, but 1 argument was provided llvm::formatv("{0} {1} {2}", 1, 2); - // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: formatv() format string requires 3 argument(s), but 2 argument(s) were provided + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: formatv() format string requires 3 arguments, but 2 arguments were provided } void too_many_args() { llvm::formatv("{0}", 1, 2); - // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: formatv() format string requires 1 argument(s), but 2 argument(s) were provided + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: formatv() format string requires 1 argument, but 2 arguments were provided llvm::formatv("no replacements", 1); - // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: formatv() format string requires 0 argument(s), but 1 argument(s) were provided + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: formatv() format string requires 0 arguments, but 1 argument was provided } void mixed_indices() { @@ -49,7 +49,7 @@ void mixed_indices() { void holes_in_indices() { llvm::formatv("{0} {2}", 1, 2, 3); - // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: formatv() format string does not use argument at index 1 + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: formatv() argument unused in format string } void non_literal_format_string(const char *fmt) { >From 1b858f1410c9f320c4587e854185cb38d3e7ed69 Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Wed, 6 May 2026 17:49:02 -0700 Subject: [PATCH 10/28] Add release note; updated other docs --- clang-tools-extra/docs/ReleaseNotes.rst | 6 ++++++ .../clang-tidy/checks/llvm/formatv-string.rst | 17 +++++++++-------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index acf000908acb2..22eab2de285fc 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -125,6 +125,12 @@ New checks Finds functions where throwing exceptions is unsafe but the function is still marked as potentially throwing. +- New :doc:`llvm-formatv-string + <clang-tidy/checks/llvm/formatv-string>` check. + + Validates ``llvm::formatv`` format strings against the provided arguments, + diagnosing mismatched argument counts, unused arguments, and mixed index styles. + - New :doc:`llvm-redundant-casting <clang-tidy/checks/llvm/redundant-casting>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst b/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst index 233c960d8eb39..96058c4e716a2 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst @@ -10,18 +10,18 @@ This check diagnoses the following issues: - The number of replacement indices in the format string does not match the number of arguments provided. -- A format string does not use an argument at a given index. -- Automatic and explicit indices are mixed (e.g. ``{} {1}``). +- A format string does not use one of the given arguments. +- Mixing of automatic and explicit indices (e.g. ``{} {1}``). .. code-block:: c++ - // Warning: requires 2 arguments, but 1 provided. + // warning: formatv() format string requires 2 arguments, but 1 argument was provided llvm::formatv("{0} {1}", x); - // Warning: mixes automatic and explicit indices. + // warning: formatv() format string mixes automatic and explicit indices llvm::formatv("{} {1}", x, y); - // Warning: format string does not use argument at index 1. + // warning: formatv() argument unused in format string llvm::formatv("{0} {2}", x, y, z); // OK. @@ -42,7 +42,8 @@ Options fully qualified function name and `index` is the zero-based parameter position of the format string. - For example, to check `mylib::log(Level, const char *Fmt, ...)` where the - format string is the second parameter (index 1): + For example, to check `mylib::log(Level, const char *Fmt, ...)` set this + option to `mylib::log:1`. The value `1` indicates the format string is found + in the second parameter. - Default is empty string. + Default is the empty string. >From c44a64c839feaa1739cd64010e3b9c793fe76428 Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Thu, 7 May 2026 11:21:19 -0700 Subject: [PATCH 11/28] Add llvm::createStringErrorV support and simplify AdditionalFunctions option --- .../clang-tidy/llvm/FormatvStringCheck.cpp | 63 +++++++++---------- .../clang-tidy/llvm/FormatvStringCheck.h | 6 +- .../clang-tidy/checks/llvm/formatv-string.rst | 16 ++--- .../llvm/formatv-string-additional.cpp | 2 +- .../checkers/llvm/formatv-string.cpp | 6 ++ 5 files changed, 48 insertions(+), 45 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp index f64e452b92838..b36d2a6049086 100644 --- a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "FormatvStringCheck.h" +#include "clang/AST/DeclTemplate.h" #include "clang/AST/Expr.h" #include "clang/ASTMatchers/ASTMatchers.h" #include "llvm/ADT/STLExtras.h" @@ -93,25 +94,17 @@ FormatvStringCheck::FormatvStringCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), AdditionalFunctions(Options.get("AdditionalFunctions", "")) { - // Always check llvm::formatv (both overloads). - Functions["llvm::formatv"] = 0; - - // Parse "name1:idx1;name2:idx2;..." from AdditionalFunctions. - llvm::StringRef Input(AdditionalFunctions); - while (!Input.empty()) { - const auto [Entry, Rest] = Input.split(';'); - Input = Rest; - if (Entry.empty()) - continue; - const auto [Name, IndexStr] = Entry.rsplit(':'); - unsigned Index = 0; - if (Name.empty() || IndexStr.empty() || IndexStr.getAsInteger(10, Index)) { - configurationDiag("invalid entry '%0' in option AdditionalFunctions, " - "expected 'fully::qualified::name:fmt_arg_index'") - << Entry; - continue; - } - Functions[Name] = Index; + Functions.insert("llvm::formatv"); + Functions.insert("llvm::createStringErrorV"); + + // Parse semicolon-separated function names from AdditionalFunctions. + const llvm::StringRef Input(AdditionalFunctions); + llvm::SmallVector<llvm::StringRef, 8> Entries; + Input.split(Entries, ';', -1, false); + for (llvm::StringRef Entry : Entries) { + Entry = Entry.trim(); + if (!Entry.empty()) + Functions.insert(Entry); } } @@ -126,7 +119,9 @@ void FormatvStringCheck::registerMatchers(MatchFinder *Finder) { llvm::copy(Functions.keys(), std::back_inserter(Names)); Finder->addMatcher( - callExpr(callee(functionDecl(hasAnyName(Names))), argumentCountAtLeast(1)) + callExpr(callee(functionDecl(hasAnyName(Names), + ast_matchers::isTemplateInstantiation())), + argumentCountAtLeast(1)) .bind("call"), this); } @@ -138,16 +133,21 @@ void FormatvStringCheck::check(const MatchFinder::MatchResult &Result) { const auto *FD = Call->getDirectCallee(); assert(FD); - // Look up the index of the format string parameter for this function. - const std::string QualName = FD->getQualifiedNameAsString(); - assert(Functions.contains(QualName) && - "matched function not in Functions map"); - unsigned FmtStringIndex = Functions.lookup(QualName); + // Find the format string index from the template signature: it's the + // parameter immediately before the trailing parameter pack. + const FunctionDecl *TemplateDecl = FD; + if (const FunctionTemplateDecl *Primary = FD->getPrimaryTemplate()) + TemplateDecl = Primary->getTemplatedDecl(); + + const unsigned NumDeclParams = TemplateDecl->getNumParams(); + if (NumDeclParams < 2) + return; + + const unsigned PackParamIndex = NumDeclParams - 1; + if (!TemplateDecl->getParamDecl(PackParamIndex)->isParameterPack()) + return; - // For llvm::formatv, also handle the (bool, const char*, ...) overload. - if (QualName == "llvm::formatv" && FD->getNumParams() > 0 && - FD->getParamDecl(0)->getType()->isBooleanType()) - FmtStringIndex = 1; + const unsigned FmtStringIndex = PackParamIndex - 1; if (Call->getNumArgs() <= FmtStringIndex) return; @@ -159,8 +159,7 @@ void FormatvStringCheck::check(const MatchFinder::MatchResult &Result) { return; const llvm::StringRef FmtString = FmtLiteral->getString(); - const unsigned FirstFmtArgIndex = FmtStringIndex + 1; - const int NumFmtArgs = Call->getNumArgs() - FirstFmtArgIndex; + const int NumFmtArgs = Call->getNumArgs() - PackParamIndex; auto ParsedOrErr = parseFormatvString(FmtString); if (!ParsedOrErr) { @@ -189,7 +188,7 @@ void FormatvStringCheck::check(const MatchFinder::MatchResult &Result) { const int UnusedIndex = UsedIndices.find_first_unset(); if (0 <= UnusedIndex && UnusedIndex < NumRequiredArgs) { // Point to the unused argument. - const Expr *UnusedArg = Call->getArg(FirstFmtArgIndex + UnusedIndex); + const Expr *UnusedArg = Call->getArg(PackParamIndex + UnusedIndex); diag(UnusedArg->getBeginLoc(), "formatv() argument unused in format string"); return; diff --git a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.h b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.h index aefd02730c12b..1d1fc8006ba8a 100644 --- a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.h +++ b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.h @@ -10,7 +10,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_FORMATVSTRINGCHECK_H #include "../ClangTidyCheck.h" -#include "llvm/ADT/StringMap.h" +#include "llvm/ADT/StringSet.h" namespace clang::tidy::llvm_check { @@ -38,9 +38,7 @@ class FormatvStringCheck : public ClangTidyCheck { } private: - // Map from fully-qualified function name to the 0-based index of the format - // string parameter. - llvm::StringMap<unsigned> Functions; + llvm::StringSet<> Functions; const std::string AdditionalFunctions; }; diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst b/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst index 96058c4e716a2..687edfb6174ac 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst @@ -37,13 +37,13 @@ Options .. option:: AdditionalFunctions - A semicolon-separated list of additional functions to check, beyond - ``llvm::formatv``. Each entry has the form `name:index`, where `name` is the - fully qualified function name and `index` is the zero-based parameter - position of the format string. - - For example, to check `mylib::log(Level, const char *Fmt, ...)` set this - option to `mylib::log:1`. The value `1` indicates the format string is found - in the second parameter. + A semicolon-separated list of additional fully qualified function names to + check, beyond ``llvm::formatv`` and ``llvm::createStringErrorV``. Each + function must be a variadic template whose last parameter is a parameter + pack. The format string is assumed to be the parameter immediately preceding + the pack. + + For example, to check ``mylib::log(Level, const char *Fmt, Ts&&...)`` set + this option to ``mylib::log``. Default is the empty string. diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-additional.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-additional.cpp index f42603ee3e19f..47b5308a64d5f 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-additional.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-additional.cpp @@ -1,5 +1,5 @@ // RUN: %check_clang_tidy %s llvm-formatv-string %t -- \ -// RUN: -config='{CheckOptions: {llvm-formatv-string.AdditionalFunctions: "mylib::log:1"}}' +// RUN: -config='{CheckOptions: {llvm-formatv-string.AdditionalFunctions: "mylib::log"}}' namespace llvm { diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp index 66707e95455bb..7971e1cf748c3 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp @@ -56,3 +56,9 @@ void non_literal_format_string(const char *fmt) { // No warning for non-literal format strings. llvm::formatv(fmt, 1, 2); } + +void bool_overload() { + llvm::formatv(false, "{0} {1}", 1, 2); + llvm::formatv(true, "{0}", 1, 2); + // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: formatv() format string requires 1 argument, but 2 arguments were provided +} >From 300161fd1bae42614ceca41281dd265edafb0f59 Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Thu, 7 May 2026 20:14:30 -0700 Subject: [PATCH 12/28] Add missing test --- .../llvm/formatv-string-autodetect.cpp | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-autodetect.cpp diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-autodetect.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-autodetect.cpp new file mode 100644 index 0000000000000..73c38423694cf --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-autodetect.cpp @@ -0,0 +1,25 @@ +// RUN: %check_clang_tidy %s llvm-formatv-string %t -- \ +// RUN: -config='{CheckOptions: {llvm-formatv-string.AdditionalFunctions: "llvm::createStringErrorV"}}' + +namespace llvm { + +template <typename... Ts> +void createStringErrorV(int EC, const char *Fmt, Ts &&...Vals) {} + +template <typename... Ts> +void createStringErrorV(const char *Fmt, Ts &&...Vals) {} + +} // namespace llvm + +void correct() { + llvm::createStringErrorV(0, "{0} {1}", 1, 2); + llvm::createStringErrorV("{0}", 42); +} + +void wrong_count() { + llvm::createStringErrorV(0, "{0} {1}", 1); + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: formatv() format string requires 2 arguments, but 1 argument was provided + + llvm::createStringErrorV("{0} {1}", 1); + // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: formatv() format string requires 2 arguments, but 1 argument was provided +} >From 377962db8b11dd2badd59a73dbe6cdd6d5981e52 Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Thu, 7 May 2026 20:19:48 -0700 Subject: [PATCH 13/28] Remove unnecessary options config in createStringErrorV test --- .../clang-tidy/checkers/llvm/formatv-string-autodetect.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-autodetect.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-autodetect.cpp index 73c38423694cf..ef0e2edd86c0c 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-autodetect.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-autodetect.cpp @@ -1,5 +1,4 @@ -// RUN: %check_clang_tidy %s llvm-formatv-string %t -- \ -// RUN: -config='{CheckOptions: {llvm-formatv-string.AdditionalFunctions: "llvm::createStringErrorV"}}' +// RUN: %check_clang_tidy %s llvm-formatv-string %t namespace llvm { >From b5cf67df1602d3ac134e2c137083b0698958a82d Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Fri, 8 May 2026 11:57:10 -0700 Subject: [PATCH 14/28] Update diagnostic messages - remove hardcoded `formatv()` --- .../clang-tidy/llvm/FormatvStringCheck.cpp | 11 +++++------ .../clang-tidy/checks/llvm/formatv-string.rst | 6 +++--- .../llvm/formatv-string-additional.cpp | 2 +- .../llvm/formatv-string-autodetect.cpp | 4 ++-- .../checkers/llvm/formatv-string.cpp | 19 ++++++++++++------- 5 files changed, 23 insertions(+), 19 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp index b36d2a6049086..72361a6c824a6 100644 --- a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp @@ -75,7 +75,8 @@ static llvm::Expected<ParseResult> parseFormatvString(llvm::StringRef Fmt) { HasAutomatic = true; } else { if (IndexStr.getAsInteger(10, Index)) - return llvm::createStringError("invalid replacement index"); + return llvm::createStringError( + "invalid replacement index in format string"); HasExplicit = true; } @@ -163,8 +164,7 @@ void FormatvStringCheck::check(const MatchFinder::MatchResult &Result) { auto ParsedOrErr = parseFormatvString(FmtString); if (!ParsedOrErr) { - diag(FmtLiteral->getBeginLoc(), "formatv() %0") - << llvm::toString(ParsedOrErr.takeError()); + diag(FmtLiteral->getBeginLoc(), llvm::toString(ParsedOrErr.takeError())); return; } @@ -173,7 +173,7 @@ void FormatvStringCheck::check(const MatchFinder::MatchResult &Result) { if (NumRequiredArgs != NumFmtArgs) { diag(FmtLiteral->getBeginLoc(), - "formatv() format string requires %0 argument%s0, but %1 argument%s1 " + "format string requires %0 argument%s0, but %1 argument%s1 " "%plural{1:was|:were}1 provided") << NumRequiredArgs << NumFmtArgs; return; @@ -189,8 +189,7 @@ void FormatvStringCheck::check(const MatchFinder::MatchResult &Result) { if (0 <= UnusedIndex && UnusedIndex < NumRequiredArgs) { // Point to the unused argument. const Expr *UnusedArg = Call->getArg(PackParamIndex + UnusedIndex); - diag(UnusedArg->getBeginLoc(), - "formatv() argument unused in format string"); + diag(UnusedArg->getBeginLoc(), "argument unused in format string"); return; } } diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst b/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst index 687edfb6174ac..14cfc035c8864 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst @@ -15,13 +15,13 @@ This check diagnoses the following issues: .. code-block:: c++ - // warning: formatv() format string requires 2 arguments, but 1 argument was provided + // warning: format string requires 2 arguments, but 1 argument was provided llvm::formatv("{0} {1}", x); - // warning: formatv() format string mixes automatic and explicit indices + // warning: format string mixes automatic and explicit indices llvm::formatv("{} {1}", x, y); - // warning: formatv() argument unused in format string + // warning: argument unused in format string llvm::formatv("{0} {2}", x, y, z); // OK. diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-additional.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-additional.cpp index 47b5308a64d5f..ee1a4dd4a43f3 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-additional.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-additional.cpp @@ -24,5 +24,5 @@ void correct() { void wrong_count() { mylib::log(mylib::Info, "{0} {1}", 1); - // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: formatv() format string requires 2 arguments, but 1 argument was provided + // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: format string requires 2 arguments, but 1 argument was provided } diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-autodetect.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-autodetect.cpp index ef0e2edd86c0c..9b4288ba9a65e 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-autodetect.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string-autodetect.cpp @@ -17,8 +17,8 @@ void correct() { void wrong_count() { llvm::createStringErrorV(0, "{0} {1}", 1); - // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: formatv() format string requires 2 arguments, but 1 argument was provided + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: format string requires 2 arguments, but 1 argument was provided llvm::createStringErrorV("{0} {1}", 1); - // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: formatv() format string requires 2 arguments, but 1 argument was provided + // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: format string requires 2 arguments, but 1 argument was provided } diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp index 7971e1cf748c3..147fed7f53106 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp @@ -28,28 +28,28 @@ void correct() { void too_few_args() { llvm::formatv("{0} {1}", 1); - // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: formatv() format string requires 2 arguments, but 1 argument was provided + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: format string requires 2 arguments, but 1 argument was provided llvm::formatv("{0} {1} {2}", 1, 2); - // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: formatv() format string requires 3 arguments, but 2 arguments were provided + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: format string requires 3 arguments, but 2 arguments were provided } void too_many_args() { llvm::formatv("{0}", 1, 2); - // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: formatv() format string requires 1 argument, but 2 arguments were provided + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: format string requires 1 argument, but 2 arguments were provided llvm::formatv("no replacements", 1); - // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: formatv() format string requires 0 arguments, but 1 argument was provided + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: format string requires 0 arguments, but 1 argument was provided } void mixed_indices() { llvm::formatv("{} {1}", 1, 2); - // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: formatv() format string mixes automatic and explicit indices + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: format string mixes automatic and explicit indices } void holes_in_indices() { llvm::formatv("{0} {2}", 1, 2, 3); - // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: formatv() argument unused in format string + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: argument unused in format string } void non_literal_format_string(const char *fmt) { @@ -60,5 +60,10 @@ void non_literal_format_string(const char *fmt) { void bool_overload() { llvm::formatv(false, "{0} {1}", 1, 2); llvm::formatv(true, "{0}", 1, 2); - // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: formatv() format string requires 1 argument, but 2 arguments were provided + // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: format string requires 1 argument, but 2 arguments were provided +} + +void invalid_index() { + llvm::formatv("{abc}", 1); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid replacement index in format string } >From ef0bdfe1cdebdca89c0f9a41924583957dee39d2 Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Fri, 8 May 2026 12:07:51 -0700 Subject: [PATCH 15/28] Address documentation feedback --- .../docs/clang-tidy/checks/llvm/formatv-string.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst b/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst index 14cfc035c8864..4eed694981de3 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst @@ -3,8 +3,8 @@ llvm-formatv-string =================== -Validates ``llvm::formatv`` format strings against the arguments provided, -similar to how the compiler validates ``printf`` format strings. +Validates ``llvm::formatv`` format strings against the provided arguments, +diagnosing mismatched argument counts, unused arguments, and mixed index styles. This check diagnoses the following issues: @@ -44,6 +44,6 @@ Options the pack. For example, to check ``mylib::log(Level, const char *Fmt, Ts&&...)`` set - this option to ``mylib::log``. + this option to `mylib::log`. Default is the empty string. >From 6d42d724266ba49f65830587e35b219dad6ad02d Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Fri, 8 May 2026 12:15:21 -0700 Subject: [PATCH 16/28] Fix rst formatting --- .../docs/clang-tidy/checks/llvm/formatv-string.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst b/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst index 4eed694981de3..ce36d935b11c7 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/llvm/formatv-string.rst @@ -4,7 +4,8 @@ llvm-formatv-string =================== Validates ``llvm::formatv`` format strings against the provided arguments, -diagnosing mismatched argument counts, unused arguments, and mixed index styles. +diagnosing mismatched argument counts, unused arguments, and mixed index +styles. This check diagnoses the following issues: >From 44b039fa508cd470685b772764af5aac6bfad8b7 Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Fri, 8 May 2026 19:21:41 -0700 Subject: [PATCH 17/28] Apply unqualified llvm:: suggestion from @localspook Co-authored-by: Victor Chernyakin <[email protected]> --- clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp index 72361a6c824a6..dafe6c110e81d 100644 --- a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp @@ -99,7 +99,7 @@ FormatvStringCheck::FormatvStringCheck(StringRef Name, Functions.insert("llvm::createStringErrorV"); // Parse semicolon-separated function names from AdditionalFunctions. - const llvm::StringRef Input(AdditionalFunctions); + const StringRef Input(AdditionalFunctions); llvm::SmallVector<llvm::StringRef, 8> Entries; Input.split(Entries, ';', -1, false); for (llvm::StringRef Entry : Entries) { >From e5b3d12da428bb6f1f67047ab8e9e137de31297e Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Fri, 8 May 2026 19:22:40 -0700 Subject: [PATCH 18/28] Apply StringRef option suggestion from @localspook Co-authored-by: Victor Chernyakin <[email protected]> --- clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.h b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.h index 1d1fc8006ba8a..36587b9fa362c 100644 --- a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.h +++ b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.h @@ -39,7 +39,7 @@ class FormatvStringCheck : public ClangTidyCheck { private: llvm::StringSet<> Functions; - const std::string AdditionalFunctions; + const StringRef AdditionalFunctions; }; } // namespace clang::tidy::llvm_check >From bd2f88264e9cb1b3791d7f85b47bbc2a5d91fe51 Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Sat, 9 May 2026 17:43:05 -0700 Subject: [PATCH 19/28] Drop llvm:: where permitted --- .../clang-tidy/llvm/FormatvStringCheck.cpp | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp index dafe6c110e81d..aeefe78a3c07b 100644 --- a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp @@ -22,13 +22,13 @@ namespace clang::tidy::llvm_check { namespace { struct ParseResult { - llvm::SmallVector<unsigned, 4> Indices; + SmallVector<unsigned, 4> Indices; unsigned MaxIndex = 0; }; } // namespace -static llvm::Expected<ParseResult> parseFormatvString(llvm::StringRef Fmt) { +static Expected<ParseResult> parseFormatvString(StringRef Fmt) { ParseResult Result; unsigned NextAutoIndex = 0; bool HasAutomatic = false; @@ -36,7 +36,7 @@ static llvm::Expected<ParseResult> parseFormatvString(llvm::StringRef Fmt) { while (!Fmt.empty()) { const size_t OpenBrace = Fmt.find('{'); - if (OpenBrace == llvm::StringRef::npos) + if (OpenBrace == StringRef::npos) break; Fmt = Fmt.drop_front(OpenBrace); @@ -49,22 +49,22 @@ static llvm::Expected<ParseResult> parseFormatvString(llvm::StringRef Fmt) { // Find the closing '}'. const size_t CloseBrace = Fmt.find('}'); - if (CloseBrace == llvm::StringRef::npos) + if (CloseBrace == StringRef::npos) return llvm::createStringError("unterminated brace in format string"); // Extract the content between braces. - const llvm::StringRef Content = Fmt.substr(1, CloseBrace - 1); + const StringRef Content = Fmt.substr(1, CloseBrace - 1); Fmt = Fmt.drop_front(CloseBrace + 1); // Parse the replacement field: [index] ["," layout] [":" format] - llvm::StringRef IndexStr = Content; + StringRef IndexStr = Content; // Strip layout and format parts for index parsing. const size_t CommaPos = Content.find(','); const size_t ColonPos = Content.find(':'); - if (CommaPos != llvm::StringRef::npos) + if (CommaPos != StringRef::npos) IndexStr = Content.substr(0, CommaPos); - else if (ColonPos != llvm::StringRef::npos) + else if (ColonPos != StringRef::npos) IndexStr = Content.substr(0, ColonPos); IndexStr = IndexStr.trim(); @@ -100,9 +100,9 @@ FormatvStringCheck::FormatvStringCheck(StringRef Name, // Parse semicolon-separated function names from AdditionalFunctions. const StringRef Input(AdditionalFunctions); - llvm::SmallVector<llvm::StringRef, 8> Entries; + SmallVector<StringRef, 8> Entries; Input.split(Entries, ';', -1, false); - for (llvm::StringRef Entry : Entries) { + for (StringRef Entry : Entries) { Entry = Entry.trim(); if (!Entry.empty()) Functions.insert(Entry); @@ -115,9 +115,9 @@ void FormatvStringCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { void FormatvStringCheck::registerMatchers(MatchFinder *Finder) { // Build a matcher for all configured function names. - std::vector<llvm::StringRef> Names; + std::vector<StringRef> Names; Names.reserve(Functions.size()); - llvm::copy(Functions.keys(), std::back_inserter(Names)); + copy(Functions.keys(), std::back_inserter(Names)); Finder->addMatcher( callExpr(callee(functionDecl(hasAnyName(Names), @@ -159,12 +159,12 @@ void FormatvStringCheck::check(const MatchFinder::MatchResult &Result) { if (!FmtLiteral) return; - const llvm::StringRef FmtString = FmtLiteral->getString(); + const StringRef FmtString = FmtLiteral->getString(); const int NumFmtArgs = Call->getNumArgs() - PackParamIndex; auto ParsedOrErr = parseFormatvString(FmtString); if (!ParsedOrErr) { - diag(FmtLiteral->getBeginLoc(), llvm::toString(ParsedOrErr.takeError())); + diag(FmtLiteral->getBeginLoc(), toString(ParsedOrErr.takeError())); return; } >From 1a28bca0f0c6b645acf066ff3cc59a1443753a08 Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Sat, 9 May 2026 17:45:58 -0700 Subject: [PATCH 20/28] Add formatv test with no arguments --- .../test/clang-tidy/checkers/llvm/formatv-string.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp index 147fed7f53106..311e1915cbfc1 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp @@ -27,6 +27,9 @@ void correct() { } void too_few_args() { + llvm::formatv("{0}"); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: format string requires 1 argument, but 0 arguments were provided + llvm::formatv("{0} {1}", 1); // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: format string requires 2 arguments, but 1 argument was provided >From 404b03fe4ccd3ec5f503e9164c65d809555a84b6 Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Sat, 9 May 2026 17:54:48 -0700 Subject: [PATCH 21/28] Update handling of AdditionalFunctions --- .../clang-tidy/llvm/FormatvStringCheck.cpp | 26 +++++++------------ .../clang-tidy/llvm/FormatvStringCheck.h | 4 +-- 2 files changed, 11 insertions(+), 19 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp index aeefe78a3c07b..a3a00150f219e 100644 --- a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "FormatvStringCheck.h" +#include "../utils/OptionsUtils.h" #include "clang/AST/DeclTemplate.h" #include "clang/AST/Expr.h" #include "clang/ASTMatchers/ASTMatchers.h" @@ -14,6 +15,8 @@ #include "llvm/ADT/SmallBitVector.h" #include "llvm/ADT/SmallVector.h" #include "llvm/Support/Error.h" +#include <iterator> +#include <vector> using namespace clang::ast_matchers; @@ -95,18 +98,11 @@ FormatvStringCheck::FormatvStringCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), AdditionalFunctions(Options.get("AdditionalFunctions", "")) { - Functions.insert("llvm::formatv"); - Functions.insert("llvm::createStringErrorV"); - - // Parse semicolon-separated function names from AdditionalFunctions. - const StringRef Input(AdditionalFunctions); - SmallVector<StringRef, 8> Entries; - Input.split(Entries, ';', -1, false); - for (StringRef Entry : Entries) { - Entry = Entry.trim(); - if (!Entry.empty()) - Functions.insert(Entry); - } + Functions = {"llvm::formatv", "llvm::createStringErrorV"}; + + std::vector<StringRef> CustomFunctions = + utils::options::parseStringList(AdditionalFunctions); + copy(CustomFunctions, std::back_inserter(Functions)); } void FormatvStringCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { @@ -115,12 +111,8 @@ void FormatvStringCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { void FormatvStringCheck::registerMatchers(MatchFinder *Finder) { // Build a matcher for all configured function names. - std::vector<StringRef> Names; - Names.reserve(Functions.size()); - copy(Functions.keys(), std::back_inserter(Names)); - Finder->addMatcher( - callExpr(callee(functionDecl(hasAnyName(Names), + callExpr(callee(functionDecl(hasAnyName(Functions), ast_matchers::isTemplateInstantiation())), argumentCountAtLeast(1)) .bind("call"), diff --git a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.h b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.h index 36587b9fa362c..d1a3db4e77f4a 100644 --- a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.h +++ b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.h @@ -10,7 +10,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_FORMATVSTRINGCHECK_H #include "../ClangTidyCheck.h" -#include "llvm/ADT/StringSet.h" +#include <vector> namespace clang::tidy::llvm_check { @@ -38,7 +38,7 @@ class FormatvStringCheck : public ClangTidyCheck { } private: - llvm::StringSet<> Functions; + std::vector<StringRef> Functions; const StringRef AdditionalFunctions; }; >From e596330f2f25ba8e9961edbf2fd4159676af2c28 Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Sat, 9 May 2026 18:38:58 -0700 Subject: [PATCH 22/28] Diagnose all unused arguments --- clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp | 7 +++---- .../test/clang-tidy/checkers/llvm/formatv-string.cpp | 4 ++++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp index a3a00150f219e..3b770d573858b 100644 --- a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp @@ -177,12 +177,11 @@ void FormatvStringCheck::check(const MatchFinder::MatchResult &Result) { for (const unsigned Index : Parsed.Indices) UsedIndices.set(Index); - const int UnusedIndex = UsedIndices.find_first_unset(); - if (0 <= UnusedIndex && UnusedIndex < NumRequiredArgs) { - // Point to the unused argument. + auto UnusedIndices = UsedIndices.flip(); + for (auto UnusedIndex : UnusedIndices.set_bits()) { + // Point to unused arguments. const Expr *UnusedArg = Call->getArg(PackParamIndex + UnusedIndex); diag(UnusedArg->getBeginLoc(), "argument unused in format string"); - return; } } } diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp index 311e1915cbfc1..3c5926a9137b0 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp @@ -53,6 +53,10 @@ void mixed_indices() { void holes_in_indices() { llvm::formatv("{0} {2}", 1, 2, 3); // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: argument unused in format string + + llvm::formatv("{2}", 1, 2, 3); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: argument unused in format string + // CHECK-MESSAGES: :[[@LINE-2]]:27: warning: argument unused in format string } void non_literal_format_string(const char *fmt) { >From 65974f6b59cd6efbecf3cb7bf52116577c8f46ad Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Sun, 10 May 2026 08:21:48 -0700 Subject: [PATCH 23/28] Apply Functions allocation suggestion from @localspook Co-authored-by: Victor Chernyakin <[email protected]> --- clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp index 3b770d573858b..c11b5a34be755 100644 --- a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp @@ -98,11 +98,9 @@ FormatvStringCheck::FormatvStringCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), AdditionalFunctions(Options.get("AdditionalFunctions", "")) { - Functions = {"llvm::formatv", "llvm::createStringErrorV"}; - - std::vector<StringRef> CustomFunctions = - utils::options::parseStringList(AdditionalFunctions); - copy(CustomFunctions, std::back_inserter(Functions)); + Functions = utils::options::parseStringList(AdditionalFunctions); + Functions.emplace_back("::llvm::formatv"); + Functions.emplace_back("::llvm::createStringErrorV"); } void FormatvStringCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { >From 96bf15e27bc94c2223e4bd063eb751e91da1e526 Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Sun, 10 May 2026 08:22:38 -0700 Subject: [PATCH 24/28] Apply unused indices suggestion from @localspook Co-authored-by: Victor Chernyakin <[email protected]> --- clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp index c11b5a34be755..c41379df12a02 100644 --- a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp @@ -171,12 +171,11 @@ void FormatvStringCheck::check(const MatchFinder::MatchResult &Result) { // Check for unused arguments. if (!Parsed.Indices.empty()) { - llvm::SmallBitVector UsedIndices(NumRequiredArgs); + llvm::SmallBitVector UnusedIndices(NumRequiredArgs, true); for (const unsigned Index : Parsed.Indices) - UsedIndices.set(Index); + UsedIndices.reset(Index); - auto UnusedIndices = UsedIndices.flip(); - for (auto UnusedIndex : UnusedIndices.set_bits()) { + for (const auto UnusedIndex : UnusedIndices.set_bits()) { // Point to unused arguments. const Expr *UnusedArg = Call->getArg(PackParamIndex + UnusedIndex); diag(UnusedArg->getBeginLoc(), "argument unused in format string"); >From a5f5f4c19de97fb685a61f333eec00fa76cf349a Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Sun, 10 May 2026 08:40:57 -0700 Subject: [PATCH 25/28] Update to unused indices handling --- .../clang-tidy/llvm/FormatvStringCheck.cpp | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp index c41379df12a02..9a3d41c73fea3 100644 --- a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp @@ -170,16 +170,14 @@ void FormatvStringCheck::check(const MatchFinder::MatchResult &Result) { } // Check for unused arguments. - if (!Parsed.Indices.empty()) { - llvm::SmallBitVector UnusedIndices(NumRequiredArgs, true); - for (const unsigned Index : Parsed.Indices) - UsedIndices.reset(Index); - - for (const auto UnusedIndex : UnusedIndices.set_bits()) { - // Point to unused arguments. - const Expr *UnusedArg = Call->getArg(PackParamIndex + UnusedIndex); - diag(UnusedArg->getBeginLoc(), "argument unused in format string"); - } + llvm::SmallBitVector UnusedIndices(NumRequiredArgs, true); + for (const unsigned Index : Parsed.Indices) + UnusedIndices.reset(Index); + + for (const auto UnusedIndex : UnusedIndices.set_bits()) { + // Point to unused arguments. + const Expr *UnusedArg = Call->getArg(PackParamIndex + UnusedIndex); + diag(UnusedArg->getBeginLoc(), "argument unused in format string"); } } >From c09d8db54c3ace440e787326c8fe9d918f6f5dfa Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Sun, 10 May 2026 08:46:45 -0700 Subject: [PATCH 26/28] Remove unused includes --- clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp index 9a3d41c73fea3..33bd077141787 100644 --- a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp @@ -11,11 +11,9 @@ #include "clang/AST/DeclTemplate.h" #include "clang/AST/Expr.h" #include "clang/ASTMatchers/ASTMatchers.h" -#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallBitVector.h" #include "llvm/ADT/SmallVector.h" #include "llvm/Support/Error.h" -#include <iterator> #include <vector> using namespace clang::ast_matchers; >From b76d1e05154a987461faa334fda08f5d5870f5ac Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Sun, 10 May 2026 09:02:04 -0700 Subject: [PATCH 27/28] Fix index parsing bug --- .../clang-tidy/llvm/FormatvStringCheck.cpp | 11 ++++++----- .../test/clang-tidy/checkers/llvm/formatv-string.cpp | 1 + 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp index 33bd077141787..a01b6c61e7a4e 100644 --- a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp @@ -58,16 +58,17 @@ static Expected<ParseResult> parseFormatvString(StringRef Fmt) { Fmt = Fmt.drop_front(CloseBrace + 1); // Parse the replacement field: [index] ["," layout] [":" format] + // Strip the format part first, since it may contain commas (e.g. {0:$[,]}). StringRef IndexStr = Content; - // Strip layout and format parts for index parsing. - const size_t CommaPos = Content.find(','); const size_t ColonPos = Content.find(':'); - if (CommaPos != StringRef::npos) - IndexStr = Content.substr(0, CommaPos); - else if (ColonPos != StringRef::npos) + if (ColonPos != StringRef::npos) IndexStr = Content.substr(0, ColonPos); + const size_t CommaPos = IndexStr.find(','); + if (CommaPos != StringRef::npos) + IndexStr = IndexStr.substr(0, CommaPos); + IndexStr = IndexStr.trim(); unsigned Index = 0; diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp index 3c5926a9137b0..fd3219823e03c 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/formatv-string.cpp @@ -19,6 +19,7 @@ void correct() { llvm::formatv("{0,-10}", 1); llvm::formatv("{0:x}", 1); llvm::formatv("{0,10:x}", 1); + llvm::formatv("{0:$[,]}", 1); llvm::formatv("no replacements"); llvm::formatv("escaped {{ braces }}"); llvm::formatv("{}", 1); >From 52bc56abfb31097920a6a97f79df7d44e1168d96 Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Sun, 10 May 2026 13:23:45 -0700 Subject: [PATCH 28/28] Apply index parsing suggestion from @localspook Co-authored-by: Victor Chernyakin <[email protected]> --- .../clang-tidy/llvm/FormatvStringCheck.cpp | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp index a01b6c61e7a4e..68eb5988ecf3d 100644 --- a/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/FormatvStringCheck.cpp @@ -59,15 +59,7 @@ static Expected<ParseResult> parseFormatvString(StringRef Fmt) { // Parse the replacement field: [index] ["," layout] [":" format] // Strip the format part first, since it may contain commas (e.g. {0:$[,]}). - StringRef IndexStr = Content; - - const size_t ColonPos = Content.find(':'); - if (ColonPos != StringRef::npos) - IndexStr = Content.substr(0, ColonPos); - - const size_t CommaPos = IndexStr.find(','); - if (CommaPos != StringRef::npos) - IndexStr = IndexStr.substr(0, CommaPos); + const StringRef IndexStr = Content.substr(0, Content.find_first_of(",:"); IndexStr = IndexStr.trim(); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
