https://github.com/kastiglione created https://github.com/llvm/llvm-project/pull/195974
None >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] [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); +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
