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

[via cfe-commits]

=?utf-8?q?Félix-Antoine?= Constantin,
=?utf-8?q?Félix-Antoine?= Constantin,
=?utf-8?q?Félix-Antoine?= Constantin,
=?utf-8?q?Félix-Antoine?= Constantin,
=?utf-8?q?Félix-Antoine?= Constantin,
=?utf-8?q?Félix-Antoine?= Constantin,
=?utf-8?q?Félix-Antoine?= Constantin,
=?utf-8?q?Félix-Antoine?= Constantin,
=?utf-8?q?Félix-Antoine?= Constantin,
=?utf-8?q?Félix-Antoine?= Constantin,
=?utf-8?q?Félix-Antoine?= Constantin
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/73069 at github.com>


================
@@ -0,0 +1,113 @@
+//===--- 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 {
+
+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");
+}
+
+static std::optional<SourceLocation>
+getInlineTokenLocation(SourceRange RangeLocation, const SourceManager &Sources,
+                       const LangOptions &LangOpts) {
+  SourceLocation Loc = RangeLocation.getBegin();
+  if (Loc.isMacroID())
+    return std::nullopt;
+
+  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 std::nullopt;
+}
+
+void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      functionDecl(isInlineSpecified(),
+                   anyOf(isConstexpr(), isDeleted(), isDefaulted(),
+                         isInAnonymousNamespace(),
----------------
FalcoGer wrote:

According to [cppref](https://en.cppreference.com/w/cpp/language/inline), static and anon namespace variables are implicitly inline when they are also constexpr.

It doesn't say anything about static functions, probably because a static, inline function makes no sense.

Statically declared functions are only visible in their TU, so inline would be redundant. Inline only allows redefinition across multiple TUs. Not sure if modules change any of that.

So technically it's pointless, not implicit. Same with templates.

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