https://github.com/higher-performance updated https://github.com/llvm/llvm-project/pull/223501
>From 1bd9e55c30756bc9e90eb358c43815e8a9a8dda0 Mon Sep 17 00:00:00 2001 From: higher-performance <[email protected]> Date: Mon, 14 Sep 2026 15:05:25 -0400 Subject: [PATCH] [clang-tidy] Make readability-redundant-inline-specifier diagnose `static inline` outside header files --- .../RedundantInlineSpecifierCheck.cpp | 60 ++++++++++++++++--- .../RedundantInlineSpecifierCheck.h | 6 +- clang-tools-extra/docs/ReleaseNotes.md | 6 ++ .../readability/redundant-inline-specifier.md | 6 ++ .../redundant-inline-specifier-header.cpp | 14 +++++ ...dant-inline-specifier-no-static-inline.cpp | 15 +++++ .../redundant-inline-specifier.cpp | 19 +++++- 7 files changed, 115 insertions(+), 11 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier-header.cpp create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier-no-static-inline.cpp diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp index e3ee9247b380d..4181634f81816 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "RedundantInlineSpecifierCheck.h" +#include "../utils/FileExtensionsUtils.h" #include "../utils/LexerUtils.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" @@ -47,6 +48,28 @@ AST_POLYMORPHIC_MATCHER_P(isInternalLinkage, return VD->isInAnonymousNamespace(); llvm_unreachable("Not a valid polymorphic type"); } + +/// Matches a non-member declaration that is spelled ``static`` and does not +/// live in a header file, where ``inline`` therefore buys nothing over plain +/// ``static``. +AST_MATCHER_P2(NamedDecl, isStaticInlineOutsideHeader, bool, + DiagnoseStaticInline, FileExtensionsSet, HeaderFileExtensions) { + // 'static' on a member means something unrelated to linkage. + if (!DiagnoseStaticInline || Node.isCXXClassMember()) + return false; + if (const auto *FD = dyn_cast<FunctionDecl>(&Node)) { + if (FD->getStorageClass() != SC_Static) + return false; + } else if (const auto *VD = dyn_cast<VarDecl>(&Node)) { + if (VD->getStorageClass() != SC_Static) + return false; + } else { + return false; + } + return !utils::isPresumedLocInHeaderFile( + Node.getLocation(), Finder->getASTContext().getSourceManager(), + HeaderFileExtensions); +} } // namespace static SourceLocation getInlineTokenLocation(SourceRange RangeLocation, @@ -71,14 +94,26 @@ static SourceLocation getInlineTokenLocation(SourceRange RangeLocation, return {}; } +void RedundantInlineSpecifierCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "StrictMode", StrictMode); + Options.store(Opts, "DiagnoseStaticInline", DiagnoseStaticInline); +} + void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) { const auto IsPartOfRecordDecl = hasAncestor(recordDecl()); + const auto IsStaticInlineOutsideHeader = + namedDecl(isStaticInlineOutsideHeader(DiagnoseStaticInline, + getHeaderFileExtensions())) + .bind("static_inline"); + Finder->addMatcher( functionDecl(isInlineSpecified(), anyOf(isConstexpr(), isDeleted(), allOf(isDefaulted(), IsPartOfRecordDecl), isInternalLinkage(StrictMode), - allOf(isDefinition(), IsPartOfRecordDecl))) + allOf(isDefinition(), IsPartOfRecordDecl), + IsStaticInlineOutsideHeader)) .bind("fun_decl"), this); @@ -96,7 +131,8 @@ void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) { anyOf(allOf(isInternalLinkage(StrictMode), unless(allOf(hasInitializer(expr()), IsPartOfRecordDecl, isStaticStorageClass()))), - allOf(isConstexpr(), IsPartOfRecordDecl))) + allOf(isConstexpr(), IsPartOfRecordDecl), + IsStaticInlineOutsideHeader)) .bind("var_decl"), this); } @@ -115,17 +151,25 @@ void RedundantInlineSpecifierCheck::handleMatchedDecl( void RedundantInlineSpecifierCheck::check( const MatchFinder::MatchResult &Result) { const SourceManager &Sources = *Result.SourceManager; + const bool IsStaticInline = + Result.Nodes.getNodeAs<Decl>("static_inline") != nullptr; if (const auto *MatchedDecl = Result.Nodes.getNodeAs<FunctionDecl>("fun_decl")) { - handleMatchedDecl( - MatchedDecl, Sources, Result, - "function %0 has inline specifier but is implicitly inlined"); + handleMatchedDecl(MatchedDecl, Sources, Result, + IsStaticInline + ? "function %0 is declared 'static inline' outside " + "of a header; use 'static' instead" + : "function %0 has inline specifier but is " + "implicitly inlined"); } else if (const auto *MatchedDecl = Result.Nodes.getNodeAs<VarDecl>("var_decl")) { - handleMatchedDecl( - MatchedDecl, Sources, Result, - "variable %0 has inline specifier but is implicitly inlined"); + handleMatchedDecl(MatchedDecl, Sources, Result, + IsStaticInline + ? "variable %0 is declared 'static inline' outside " + "of a header; use 'static' instead" + : "variable %0 has inline specifier but is " + "implicitly inlined"); } else if (const auto *MatchedDecl = Result.Nodes.getNodeAs<FunctionTemplateDecl>("templ_decl")) { handleMatchedDecl( diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h index 5e819e700fd16..7ea2f93450a8e 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h +++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h @@ -22,7 +22,9 @@ class RedundantInlineSpecifierCheck : public ClangTidyCheck { public: RedundantInlineSpecifierCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), - StrictMode(Options.get("StrictMode", false)) {} + StrictMode(Options.get("StrictMode", false)), + DiagnoseStaticInline(Options.get("DiagnoseStaticInline", true)) {} + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; std::optional<TraversalKind> getCheckTraversalKind() const override { @@ -34,7 +36,9 @@ class RedundantInlineSpecifierCheck : public ClangTidyCheck { void handleMatchedDecl(const T *MatchedDecl, const SourceManager &Sources, const ast_matchers::MatchFinder::MatchResult &Result, StringRef Message); + const bool StrictMode; + const bool DiagnoseStaticInline; }; } // namespace clang::tidy::readability diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md index f523c7b4dc4a9..567e17ae803ba 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -270,6 +270,12 @@ infrastructure are described first, followed by tool-specific sections. exclusively for overload resolution. Added the {option}`IgnoredTypes` option to allow customizing the set of ignored types. +- Improved {doc}`readability-redundant-inline-specifier + <clang-tidy/checks/readability/redundant-inline-specifier>` check by flagging + `static inline` declarations outside of header files and suggesting `static` + alone. This is enabled by default and can be turned off with the new + {option}`DiagnoseStaticInline`. + - Improved {doc}`readability-trailing-comma <clang-tidy/checks/readability/trailing-comma>` check: diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.md b/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.md index 1f7ac0cd06666..432180fc6620b 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.md +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.md @@ -29,3 +29,9 @@ defined entirely inside a class/struct/union definition are implicitly inlined. When `true`, the check will also flag functions and variables that already have internal linkage as redundant. Default is `false`. ``` + +```{option} DiagnoseStaticInline + +If set to `true`, the check will flag `static inline` symbols declared +outside of header files and suggest dropping the `inline` specifier. +Default is `true`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier-header.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier-header.cpp new file mode 100644 index 0000000000000..6b91dbe7f28aa --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier-header.cpp @@ -0,0 +1,14 @@ +// RUN: %check_clang_tidy -std=c++17-or-later %s -assume-filename=redundant-inline-specifier-header.hpp readability-redundant-inline-specifier %t + +// OK -- not redundant in a header file. +static inline int fn0(int i) +{ + return i - 1; +} + +static inline int STATIC_INLINE_VAR = 42; + +// Redundant in header file as well as implementation file. +constexpr inline void fn1() {} +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: function 'fn1' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +// CHECK-FIXES: constexpr void fn1() {} diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier-no-static-inline.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier-no-static-inline.cpp new file mode 100644 index 0000000000000..7e29254da1ba3 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier-no-static-inline.cpp @@ -0,0 +1,15 @@ +// RUN: %check_clang_tidy -std=c++17-or-later %s readability-redundant-inline-specifier %t -- -config="{CheckOptions: {readability-redundant-inline-specifier.DiagnoseStaticInline: 'false'}}" + +// With DiagnoseStaticInline disabled, 'static inline' is left alone. +static inline int fn0(int i) +{ + return i - 1; +} + +static inline int STATIC_INLINE_VAR = 42; + +// Declarations that are redundantly inline for another reason are still +// reported. +constexpr inline void fn1() {} +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: function 'fn1' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +// CHECK-FIXES: constexpr void fn1() {} diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp index 4bd97df154b5c..e9b63909918ca 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp @@ -1,4 +1,4 @@ -// RUN: %check_clang_tidy -std=c++17-or-later %s readability-redundant-inline-specifier %t +// RUN: %check_clang_tidy -std=c++17-or-later -check-suffixes=,STATIC %s readability-redundant-inline-specifier %t // RUN: %check_clang_tidy -std=c++17-or-later -check-suffixes=,STRICT %s readability-redundant-inline-specifier %t -- -config="{CheckOptions: {readability-redundant-inline-specifier.StrictMode: 'true'}}" template <typename T> inline T f() @@ -61,10 +61,13 @@ constexpr inline int Get42() { return 42; } static constexpr inline int NAMESPACE_STATIC = 42; +// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: variable 'NAMESPACE_STATIC' is declared 'static inline' outside of a header; use 'static' instead [readability-redundant-inline-specifier] +// CHECK-FIXES: static constexpr int NAMESPACE_STATIC = 42; inline static int fn0(int i) // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:1: warning: function 'fn0' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] -// CHECK-FIXES-STRICT: static int fn0(int i) +// CHECK-MESSAGES-STATIC: :[[@LINE-2]]:1: warning: function 'fn0' is declared 'static inline' outside of a header; use 'static' instead [readability-redundant-inline-specifier] +// CHECK-FIXES: static int fn0(int i) { return i - 1; } @@ -159,3 +162,15 @@ class B inline B::~B() = default; } + +static inline int fn11(int i) +// CHECK-MESSAGES-STRICT: :[[@LINE-1]]:8: warning: function 'fn11' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +// CHECK-MESSAGES-STATIC: :[[@LINE-2]]:8: warning: function 'fn11' is declared 'static inline' outside of a header; use 'static' instead [readability-redundant-inline-specifier] +// CHECK-FIXES: static int fn11(int i) +{ + return i - 1; +} + +static inline int STATIC_INLINE_VAR = 42; +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: variable 'STATIC_INLINE_VAR' is declared 'static inline' outside of a header; use 'static' instead [readability-redundant-inline-specifier] +// CHECK-FIXES: static int STATIC_INLINE_VAR = 42; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
