fixup! [clang-tidy] Added check to detect redundant inline keyword

Added StrictMode option to decide if we want to flag redundant inline specifiers rather than implicit ones

Author: felix642

clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp

@@ -24,6 +24,7 @@ using namespace clang::ast_matchers;
 
 namespace clang::tidy::readability {
 
+namespace {
 AST_POLYMORPHIC_MATCHER(isInlineSpecified,
                         AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl,
                                                         VarDecl)) {
@@ -34,12 +35,26 @@ AST_POLYMORPHIC_MATCHER(isInlineSpecified,
   llvm_unreachable("Not a valid polymorphic type");
 }
 
-static std::optional<SourceLocation>
-getInlineTokenLocation(SourceRange RangeLocation, const SourceManager &Sources,
-                       const LangOptions &LangOpts) {
+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 std::nullopt;
+    return {};
 
   Token FirstToken;
   Lexer::getRawToken(Loc, FirstToken, Sources, LangOpts, true);
@@ -53,25 +68,29 @@ getInlineTokenLocation(SourceRange RangeLocation, const SourceManager &Sources,
     CurrentToken =
         Lexer::findNextToken(CurrentToken->getLocation(), Sources, LangOpts);
   }
-  return std::nullopt;
+  return {};
 }
 
 void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
       functionDecl(isInlineSpecified(),
                    anyOf(isConstexpr(), isDeleted(), isDefaulted(),
-                         isInAnonymousNamespace(),
+                         isInternalLinkage(StrictMode),
                          allOf(isDefinition(), hasAncestor(recordDecl()))))
           .bind("fun_decl"),
       this);
-  Finder->addMatcher(
-      functionTemplateDecl(has(functionDecl(isInlineSpecified())))
-          .bind("templ_decl"),
-      this);
+
+  if (StrictMode)
+    Finder->addMatcher(
+        functionTemplateDecl(
+            has(functionDecl(allOf(isInlineSpecified(), isDefinition()))))
+            .bind("templ_decl"),
+        this);
+
   if (getLangOpts().CPlusPlus17) {
     Finder->addMatcher(
         varDecl(isInlineSpecified(),
-                anyOf(isInAnonymousNamespace(),
+                anyOf(isInternalLinkage(StrictMode),
                       allOf(isConstexpr(), hasAncestor(recordDecl()))))
             .bind("var_decl"),
         this);
@@ -82,10 +101,10 @@ template <typename T>
 void RedundantInlineSpecifierCheck::handleMatchedDecl(
     const T *MatchedDecl, const SourceManager &Sources,
     const MatchFinder::MatchResult &Result, StringRef Message) {
-  if (std::optional<SourceLocation> Loc =
-          getInlineTokenLocation(MatchedDecl->getSourceRange(), Sources,
-                                 Result.Context->getLangOpts()))
-    diag(*Loc, Message) << MatchedDecl << FixItHint::CreateRemoval(*Loc);
+  SourceLocation Loc = getInlineTokenLocation(
+      MatchedDecl->getSourceRange(), Sources, Result.Context->getLangOpts());
+  if (Loc.isValid())
+    diag(Loc, Message) << MatchedDecl << FixItHint::CreateRemoval(Loc);
 }
 
 void RedundantInlineSpecifierCheck::check(

clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h

@@ -21,7 +21,8 @@ namespace clang::tidy::readability {
 class RedundantInlineSpecifierCheck : public ClangTidyCheck {
 public:
   RedundantInlineSpecifierCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, 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 {
@@ -33,6 +34,7 @@ class RedundantInlineSpecifierCheck : public ClangTidyCheck {
   void handleMatchedDecl(const T *MatchedDecl, const SourceManager &Sources,
                          const ast_matchers::MatchFinder::MatchResult &Result,
                          StringRef Message);
+  const bool StrictMode;
 };
 
 } // namespace clang::tidy::readability

clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.rst

@@ -22,3 +22,11 @@ functions are implicitly inlined
 
 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.

clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp

@@ -1,8 +1,9 @@
-// RUN: %check_clang_tidy %s readability-redundant-inline-specifier -std=c++17 %t
+// 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: :[[@LINE-1]]:23: warning: function 'f' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
-// CHECK-FIXES: template <typename T> 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{};
 }
@@ -12,7 +13,6 @@ template <> inline double f<double>() = delete;
 // 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);
 }
@@ -30,6 +30,7 @@ class C
 
     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]
@@ -52,7 +53,6 @@ class C
     // 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; }
@@ -61,10 +61,10 @@ constexpr inline 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]
+// 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;
 }
@@ -79,7 +79,8 @@ static constexpr inline int fn1(int i)
 namespace
 {
     inline int fn2(int i)
-    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'fn2' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
+    // 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;
     }
@@ -92,13 +93,13 @@ namespace
     }
 
     inline constexpr int MY_CONSTEXPR_VAR = 42;
-    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'MY_CONSTEXPR_VAR' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
+    // 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)
-    // CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: function 'fn4' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
     {
         return i - 1;
     }
@@ -112,24 +113,19 @@ namespace ns
 }
 
 auto fn6 = [](){};
-//CHECK-MESSAGES-NOT: :[[@LINE-1]]:1: warning: function 'operator()' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
 
 template <typename T> inline T fn7();
-// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: function 'fn7' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
-// CHECK-FIXES: template <typename T> T fn7();
 
-template <typename T>  T fn7()
-// CHECK-MESSAGES-NOT: :[[@LINE-1]]:1: warning: function 'fn7' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
+template <typename T> T fn7()
 {
     return T{};
 }
 
 template <typename T>  T fn8();
-// CHECK-MESSAGES-NOT: :[[@LINE-1]]:1: warning: function 'fn8' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
 
 template <typename T> inline T fn8()
-// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: function 'fn8' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
-// CHECK-FIXES: template <typename T> 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{};
 }