[clang-tools-extra] [clang] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)

via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 21 18:30:54 PST 2023


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang

Author: Félix-Antoine Constantin (felix642)

<details>
<summary>Changes</summary>

This checks find usages of the inline keywork where it is already implicitly defined by the compiler and suggests it's removal.

Fixes #<!-- -->72397

---
Full diff: https://github.com/llvm/llvm-project/pull/73069.diff


9 Files Affected:

- (modified) clang-tools-extra/clang-tidy/readability/CMakeLists.txt (+1) 
- (modified) clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp (+3) 
- (added) clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp (+94) 
- (added) clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h (+36) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5) 
- (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1) 
- (added) clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.rst (+34) 
- (added) clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp (+110) 
- (modified) clang/include/clang/ASTMatchers/ASTMatchers.h (+1-1) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
index 5452c2d48a46173..811310db8c721a6 100644
--- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -20,6 +20,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 b8e6e6414320600..3ce7bfecaecba64 100644
--- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -39,6 +39,7 @@
 #include "RedundantControlFlowCheck.h"
 #include "RedundantDeclarationCheck.h"
 #include "RedundantFunctionPtrDereferenceCheck.h"
+#include "RedundantInlineSpecifierCheck.h"
 #include "RedundantMemberInitCheck.h"
 #include "RedundantPreprocessorCheck.h"
 #include "RedundantSmartptrGetCheck.h"
@@ -93,6 +94,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..7b67a7419c708b7
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
@@ -0,0 +1,94 @@
+//===--- 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 "../utils/LexerUtils.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+static std::optional<SourceLocation>
+getInlineTokenLocation(SourceRange RangeLocation,
+                       const SourceManager &Sources,
+                       const LangOptions &LangOpts) {
+  SourceLocation CurrentLocation = RangeLocation.getBegin();
+  Token CurrentToken;
+  while (!Lexer::getRawToken(CurrentLocation, CurrentToken, Sources, LangOpts,
+                             true) && CurrentLocation < RangeLocation.getEnd() && CurrentToken.isNot(tok::eof)) {
+    if (CurrentToken.is(tok::raw_identifier)) {
+      if (CurrentToken.getRawIdentifier() == "inline") {
+        return CurrentToken.getLocation();
+      }
+    }
+    CurrentLocation = CurrentToken.getEndLoc();
+  }
+  return std::nullopt;
+}
+
+void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      functionDecl(unless(isExpansionInSystemHeader()), unless(isImplicit()),
+                   unless(hasAncestor(lambdaExpr())), isInline(),
+                   anyOf(isConstexpr(), isDeleted(),
+                         allOf(isDefinition(), hasAncestor(recordDecl()),
+                               unless(hasAncestor(cxxConstructorDecl())))))
+          .bind("fun_decl"),
+      this);
+
+  Finder->addMatcher(
+      varDecl(isInline(), unless(isImplicit()),
+              anyOf(allOf(isConstexpr(), unless(isStaticStorageClass())),
+                    hasAncestor(recordDecl())))
+          .bind("var_decl"),
+      this);
+
+  Finder->addMatcher(
+    functionTemplateDecl(has(functionDecl(isInline()))).bind("templ_decl"), this
+  );
+}
+
+template <typename T>
+void RedundantInlineSpecifierCheck::handleMatchedDecl(const T *MatchedDecl, const SourceManager &Sources,
+                       const MatchFinder::MatchResult &Result,
+                       const char *Message) {
+  if (auto Loc = getInlineTokenLocation(MatchedDecl->getSourceRange(),
+                                        Sources, Result.Context->getLangOpts()))
+    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..342e56044a42b1d
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h
@@ -0,0 +1,36 @@
+//===--- 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 {
+
+/// Finds redundant `inline` specifiers in function and variable declarations.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability/readability-redundant-inline-specifier.html
+class RedundantInlineSpecifierCheck : public ClangTidyCheck {
+public:
+  RedundantInlineSpecifierCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+template <typename T>
+void handleMatchedDecl(const T *MatchedDecl, const SourceManager &Sources,
+                       const ast_matchers::MatchFinder::MatchResult &Result,
+                       const char *Message);
+};
+
+} // 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 f49c412118e7d98..2b09f4fd8a0f65c 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -192,6 +192,11 @@ New checks
   Recommends the smallest possible underlying type for an ``enum`` or ``enum``
   class based on the range of its enumerators.
 
+- New :doc:`readability-redundant-inline-specifier
+  <clang-tidy/checks/readability/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 6f987ba1672e3f2..b95d8ed64fa180d 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -348,6 +348,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/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..a066f137e0a71c2
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.rst
@@ -0,0 +1,34 @@
+.. title:: clang-tidy - readability-redundant-inline-specifier
+
+readability-redundant-inline-specifier
+=================
+
+Checks for instances of the `inline` keyword in code where it is redundant
+and recommends its removal.
+
+Examples:
+
+.. code-block:: c++
+
+   constexpr inline void f() {}
+
+In the example abvove 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.
+
+The token `inline` is considered redundant in the following cases:
+
+- When it is used in a function definition that is constexpr.
+- When it is used in a member function definition that is defined entirely
+  inside a class/struct/union definition.
+- When it is used on a deleted function. 
+- When it is used on a template declaration.
+- When it is used on a member variable that is constexpr and static.
+
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..b9b143bee46f2a9
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp
@@ -0,0 +1,110 @@
+// RUN: %check_clang_tidy %s readability-redundant-inline-specifier %t
+
+template <typename T> inline T f()
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: Function 'f' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
+// CHECK-FIXES: 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)
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:1: warning: Function 'g' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
+{
+    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;
+
+    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;
+    // CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: Variable 'C_STATIC_2' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
+};
+
+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;
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:18: warning: Variable 'NAMESPACE_STATIC' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
+
+inline static int fn0(int i)
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:1: warning: Function 'fn0' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
+{
+    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-NOT: :[[@LINE-1]]:5: warning: Function 'fn2' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
+    {
+        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;
+    }
+}
+
+namespace ns
+{
+    inline int fn4(int i)
+    // CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: Function 'fn4' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
+    {
+        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 = [](){};
+//CHECK-MESSAGES-NOT: :[[@LINE-1]]:1: warning: Function 'operator()' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
+
diff --git a/clang/include/clang/ASTMatchers/ASTMatchers.h b/clang/include/clang/ASTMatchers/ASTMatchers.h
index 82a26356c58f556..06e784649a975bf 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -7889,7 +7889,7 @@ AST_POLYMORPHIC_MATCHER(isInline, AST_POLYMORPHIC_SUPPORTED_TYPES(NamespaceDecl,
   if (const auto *NSD = dyn_cast<NamespaceDecl>(&Node))
     return NSD->isInline();
   if (const auto *VD = dyn_cast<VarDecl>(&Node))
-    return VD->isInline();
+    return VD->isInlineSpecified();
   llvm_unreachable("Not a valid polymorphic type");
 }
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/73069


More information about the cfe-commits mailing list