Author: Félix-Antoine Constantin Date: 2024-01-20T10:45:59+01:00 New Revision: 7a8f5d97afbff948fa2437c81d08a5963ee7d8cf
URL: https://github.com/llvm/llvm-project/commit/7a8f5d97afbff948fa2437c81d08a5963ee7d8cf DIFF: https://github.com/llvm/llvm-project/commit/7a8f5d97afbff948fa2437c81d08a5963ee7d8cf.diff LOG: [clang-tidy] Added new check to detect redundant inline keyword (#73069) This checks find usages of the inline keywork where it is already implicitly defined by the compiler and suggests it's removal. Fixes #72397 Added: clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.rst clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp Modified: clang-tools-extra/clang-tidy/readability/CMakeLists.txt clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/list.rst Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 636860f54c73cc..a6c8cbd8eb448a 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -22,6 +22,7 @@ add_clang_library(clangTidyReadabilityModule IdentifierLengthCheck.cpp IdentifierNamingCheck.cpp ImplicitBoolConversionCheck.cpp + RedundantInlineSpecifierCheck.cpp InconsistentDeclarationParameterNameCheck.cpp IsolateDeclarationCheck.cpp MagicNumbersCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index 2f5d47546197f7..87b299bf1ef1c5 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -42,6 +42,7 @@ #include "RedundantControlFlowCheck.h" #include "RedundantDeclarationCheck.h" #include "RedundantFunctionPtrDereferenceCheck.h" +#include "RedundantInlineSpecifierCheck.h" #include "RedundantMemberInitCheck.h" #include "RedundantPreprocessorCheck.h" #include "RedundantSmartptrGetCheck.h" @@ -100,6 +101,8 @@ class ReadabilityModule : public ClangTidyModule { "readability-identifier-naming"); CheckFactories.registerCheck<ImplicitBoolConversionCheck>( "readability-implicit-bool-conversion"); + CheckFactories.registerCheck<RedundantInlineSpecifierCheck>( + "readability-redundant-inline-specifier"); CheckFactories.registerCheck<InconsistentDeclarationParameterNameCheck>( "readability-inconsistent-declaration-parameter-name"); CheckFactories.registerCheck<IsolateDeclarationCheck>( diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp new file mode 100644 index 00000000000000..0e8d17d4442478 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp @@ -0,0 +1,132 @@ +//===--- RedundantInlineSpecifierCheck.cpp - clang-tidy--------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "RedundantInlineSpecifierCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" +#include "clang/AST/ExprCXX.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" +#include "clang/Lex/Token.h" + +#include "../utils/LexerUtils.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +namespace { +AST_POLYMORPHIC_MATCHER(isInlineSpecified, + AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl, + VarDecl)) { + if (const auto *FD = dyn_cast<FunctionDecl>(&Node)) + return FD->isInlineSpecified(); + if (const auto *VD = dyn_cast<VarDecl>(&Node)) + return VD->isInlineSpecified(); + llvm_unreachable("Not a valid polymorphic type"); +} + +AST_POLYMORPHIC_MATCHER_P(isInternalLinkage, + AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl, + VarDecl), + bool, strictMode) { + if (!strictMode) + return false; + if (const auto *FD = dyn_cast<FunctionDecl>(&Node)) + return FD->getStorageClass() == SC_Static || FD->isInAnonymousNamespace(); + if (const auto *VD = dyn_cast<VarDecl>(&Node)) + return VD->isInAnonymousNamespace(); + llvm_unreachable("Not a valid polymorphic type"); +} +} // namespace + +static SourceLocation getInlineTokenLocation(SourceRange RangeLocation, + const SourceManager &Sources, + const LangOptions &LangOpts) { + SourceLocation Loc = RangeLocation.getBegin(); + if (Loc.isMacroID()) + return {}; + + Token FirstToken; + Lexer::getRawToken(Loc, FirstToken, Sources, LangOpts, true); + std::optional<Token> CurrentToken = FirstToken; + while (CurrentToken && CurrentToken->getLocation() < RangeLocation.getEnd() && + CurrentToken->isNot(tok::eof)) { + if (CurrentToken->is(tok::raw_identifier) && + CurrentToken->getRawIdentifier() == "inline") + return CurrentToken->getLocation(); + + CurrentToken = + Lexer::findNextToken(CurrentToken->getLocation(), Sources, LangOpts); + } + return {}; +} + +void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + functionDecl(isInlineSpecified(), + anyOf(isConstexpr(), isDeleted(), isDefaulted(), + isInternalLinkage(StrictMode), + allOf(isDefinition(), hasAncestor(recordDecl())))) + .bind("fun_decl"), + this); + + if (StrictMode) + Finder->addMatcher( + functionTemplateDecl( + has(functionDecl(allOf(isInlineSpecified(), isDefinition())))) + .bind("templ_decl"), + this); + + if (getLangOpts().CPlusPlus17) { + Finder->addMatcher( + varDecl(isInlineSpecified(), + anyOf(isInternalLinkage(StrictMode), + allOf(isConstexpr(), hasAncestor(recordDecl())))) + .bind("var_decl"), + this); + } +} + +template <typename T> +void RedundantInlineSpecifierCheck::handleMatchedDecl( + const T *MatchedDecl, const SourceManager &Sources, + const MatchFinder::MatchResult &Result, StringRef Message) { + SourceLocation Loc = getInlineTokenLocation( + MatchedDecl->getSourceRange(), Sources, Result.Context->getLangOpts()); + if (Loc.isValid()) + diag(Loc, Message) << MatchedDecl << FixItHint::CreateRemoval(Loc); +} + +void RedundantInlineSpecifierCheck::check( + const MatchFinder::MatchResult &Result) { + const SourceManager &Sources = *Result.SourceManager; + + if (const auto *MatchedDecl = + Result.Nodes.getNodeAs<FunctionDecl>("fun_decl")) { + handleMatchedDecl( + MatchedDecl, Sources, Result, + "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"); + } else if (const auto *MatchedDecl = + Result.Nodes.getNodeAs<FunctionTemplateDecl>("templ_decl")) { + handleMatchedDecl( + MatchedDecl, Sources, Result, + "function %0 has inline specifier but is implicitly inlined"); + } +} + +} // namespace clang::tidy::readability diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h new file mode 100644 index 00000000000000..cc0bfa9685d0c3 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h @@ -0,0 +1,42 @@ +//===--- RedundantInlineSpecifierCheck.h - clang-tidy ------------*-C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTINLINESPECIFIERCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTINLINESPECIFIERCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::readability { + +/// Detects redundant ``inline`` specifiers on function and variable +/// declarations. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/readability/redundant-inline-specifier.html +class RedundantInlineSpecifierCheck : public ClangTidyCheck { +public: + RedundantInlineSpecifierCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + StrictMode(Options.getLocalOrGlobal("StrictMode", false)) {} + 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: + template <typename T> + void handleMatchedDecl(const T *MatchedDecl, const SourceManager &Sources, + const ast_matchers::MatchFinder::MatchResult &Result, + StringRef Message); + const bool StrictMode; +}; + +} // namespace clang::tidy::readability + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTINLINESPECIFIERCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index ed8d01d65542f9..c8e93231fd11b8 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -240,6 +240,11 @@ New checks Detects explicit type casting operations that involve the same source and destination types, and subsequently recommend their removal. + +- New :doc:`readability-redundant-inline-specifier + <clang-tidy/checks/readability/redundant-inline-specifier>` check. + + Detects redundant ``inline`` specifiers on function and variable declarations. - New :doc:`readability-reference-to-constructed-temporary <clang-tidy/checks/readability/reference-to-constructed-temporary>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 503b9fac827320..e972e376ebc5a8 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -354,6 +354,7 @@ Clang-Tidy Checks :doc:`readability-identifier-length <readability/identifier-length>`, :doc:`readability-identifier-naming <readability/identifier-naming>`, "Yes" :doc:`readability-implicit-bool-conversion <readability/implicit-bool-conversion>`, "Yes" + :doc:`readability-redundant-inline-specifier <readability/redundant-inline-specifier>`, "Yes" :doc:`readability-inconsistent-declaration-parameter-name <readability/inconsistent-declaration-parameter-name>`, "Yes" :doc:`readability-isolate-declaration <readability/isolate-declaration>`, "Yes" :doc:`readability-magic-numbers <readability/magic-numbers>`, diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.rst new file mode 100644 index 00000000000000..eee324cddab481 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.rst @@ -0,0 +1,32 @@ +.. title:: clang-tidy - readability-redundant-inline-specifier + +readability-redundant-inline-specifier +====================================== + +Detects redundant ``inline`` specifiers on function and variable declarations. + +Examples: + +.. code-block:: c++ + + constexpr inline void f() {} + +In the example above the keyword ``inline`` is redundant since constexpr +functions are implicitly inlined + +.. code-block:: c++ + + class MyClass { + inline void myMethod() {} + }; + +In the example above the keyword ``inline`` is redundant since member functions +defined entirely inside a class/struct/union definition are implicitly inlined. + +Options +------- + +.. option:: StrictMode + + If set to `true`, the check will also flag functions and variables that + already have internal linkage as redundant. 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 new file mode 100644 index 00000000000000..cdd98d8fdc20f5 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp @@ -0,0 +1,137 @@ +// RUN: %check_clang_tidy -std=c++17 %s readability-redundant-inline-specifier %t +// RUN: %check_clang_tidy -std=c++17 -check-suffixes=,STRICT %s readability-redundant-inline-specifier %t -- -config="{CheckOptions: {readability-redundant-inline-specifier.StrictMode: 'true'}}" + +template <typename T> inline T f() +// CHECK-MESSAGES-STRICT: :[[@LINE-1]]:23: warning: function 'f' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +// CHECK-FIXES-STRICT: template <typename T> T f() +{ + return T{}; +} + +template <> inline double f<double>() = delete; +// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function 'f<double>' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +// CHECK-FIXES: template <> double f<double>() = delete; + +inline int g(float a) +{ + return static_cast<int>(a - 5.F); +} + +inline int g(double) = delete; +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: function 'g' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +// CHECK-FIXES: int g(double) = delete; + +class C +{ + public: + inline C& operator=(const C&) = delete; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'operator=' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] + // CHECK-FIXES: C& operator=(const C&) = delete; + + inline C(const C&) = default; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'C' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] + // CHECK-FIXES: C(const C&) = default; + + constexpr inline C& operator=(int a); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: function 'operator=' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] + // CHECK-FIXES: constexpr C& operator=(int a); + + inline C() {} + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'C' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] + // CHECK-FIXES: C() {} + + constexpr inline C(int); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: function 'C' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] + // CHECK-FIXES: constexpr C(int); + + inline int Get42() const { return 42; } + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'Get42' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] + // CHECK-FIXES: int Get42() const { return 42; } + + static inline constexpr int C_STATIC = 42; + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'C_STATIC' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] + // CHECK-FIXES: static constexpr int C_STATIC = 42; + + static constexpr int C_STATIC_2 = 42; +}; + +constexpr inline int Get42() { return 42; } +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: function 'Get42' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +// CHECK-FIXES: constexpr int Get42() { return 42; } + + +static constexpr inline 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) +{ + return i - 1; +} + +static constexpr inline int fn1(int i) +// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: function 'fn1' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +// CHECK-FIXES: static constexpr int fn1(int i) +{ + return i - 1; +} + +namespace +{ + inline int fn2(int i) + // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:5: warning: function 'fn2' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] + // CHECK-FIXES-STRICT: int fn2(int i) + { + return i - 1; + } + + inline constexpr int fn3(int i) + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'fn3' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] + // CHECK-FIXES: constexpr int fn3(int i) + { + return i - 1; + } + + inline constexpr int MY_CONSTEXPR_VAR = 42; + // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:5: warning: variable 'MY_CONSTEXPR_VAR' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] + // CHECK-FIXES-STRICT: constexpr int MY_CONSTEXPR_VAR = 42; +} + +namespace ns +{ + inline int fn4(int i) + { + return i - 1; + } + + inline constexpr int fn5(int i) + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'fn5' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] + // CHECK-FIXES: constexpr int fn5(int i) + { + return i - 1; + } +} + +auto fn6 = [](){}; + +template <typename T> inline T fn7(); + +template <typename T> T fn7() +{ + return T{}; +} + +template <typename T> T fn8(); + +template <typename T> inline T fn8() +// CHECK-MESSAGES-STRICT: :[[@LINE-1]]:23: warning: function 'fn8' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier] +// CHECK-FIXES-STRICT: template <typename T> T fn8() +{ + return T{}; +} + +#define INLINE_MACRO() inline void fn9() { } +INLINE_MACRO() + +#define INLINE_KW inline +INLINE_KW void fn10() { } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits