[clang-tools-extra] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)
Félix-Antoine Constantin via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 19 17:40:14 PST 2024
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/73069
>From 286c4445f8cba6ea2f49fb9e8f732f04ebdb6c97 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
<felix-antoine.constantin at polymtl.ca>
Date: Thu, 16 Nov 2023 22:03:15 -0500
Subject: [PATCH] =?UTF-8?q?[clang-tidy]=C2=A0Added=20check=20to=20detect?=
=?UTF-8?q?=20redundant=20inline=20keyword?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This checks find usages of the inline keywork where it is
already implicitly defined by the compiler and suggests it's removal.
Fixes #72397
---
.../clang-tidy/readability/CMakeLists.txt | 1 +
.../readability/ReadabilityTidyModule.cpp | 3 +
.../RedundantInlineSpecifierCheck.cpp | 132 +++++++++++++++++
.../RedundantInlineSpecifierCheck.h | 42 ++++++
clang-tools-extra/docs/ReleaseNotes.rst | 5 +
.../docs/clang-tidy/checks/list.rst | 1 +
.../redundant-inline-specifier.rst | 32 ++++
.../redundant-inline-specifier.cpp | 137 ++++++++++++++++++
8 files changed, 353 insertions(+)
create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h
create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.rst
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp
diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
index fa571d5dd7650d1..1d15228da694510 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 f769752c5de5fa7..521dacd6f9df3e2 100644
--- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -41,6 +41,7 @@
#include "RedundantControlFlowCheck.h"
#include "RedundantDeclarationCheck.h"
#include "RedundantFunctionPtrDereferenceCheck.h"
+#include "RedundantInlineSpecifierCheck.h"
#include "RedundantMemberInitCheck.h"
#include "RedundantPreprocessorCheck.h"
#include "RedundantSmartptrGetCheck.h"
@@ -99,6 +100,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 000000000000000..0e8d17d4442478c
--- /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 000000000000000..cc0bfa9685d0c3f
--- /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 a235a7d02592e83..50101b4609bebcc 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -235,6 +235,11 @@ New checks
Finds return statements with ``void`` values used within functions with
``void`` result types.
+- 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 5f21449cfc3df49..02a5c3cf8536fbd 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 000000000000000..eee324cddab4819
--- /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 000000000000000..cdd98d8fdc20f57
--- /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() { }
More information about the cfe-commits
mailing list