Skip to content

Commit 3fadac1

Browse files
felix642felix642
committed
fixup! [clang-tidy] Added check to detect redundant inline keyword
Code review - Fixed a few nits
1 parent e91d16f commit 3fadac1

File tree

5 files changed

+62
-59
lines changed

5 files changed

+62
-59
lines changed

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

Lines changed: 33 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
//===--- RedundantInlineSpecifierCheck.cpp -
2-
// clang-tidy----------------------===//
1+
//===--- RedundantInlineSpecifierCheck.cpp - clang-tidy--------------------===//
32
//
43
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
54
// See https://llvm.org/LICENSE.txt for license information.
@@ -17,59 +16,70 @@
1716
#include "clang/Basic/Diagnostic.h"
1817
#include "clang/Basic/SourceLocation.h"
1918
#include "clang/Basic/SourceManager.h"
19+
#include "clang/Lex/Token.h"
2020

2121
#include "../utils/LexerUtils.h"
2222

2323
using namespace clang::ast_matchers;
2424

2525
namespace clang::tidy::readability {
2626

27+
AST_POLYMORPHIC_MATCHER(isInlineSpecified, AST_POLYMORPHIC_SUPPORTED_TYPES(
28+
FunctionDecl,
29+
VarDecl)) {
30+
if (const auto *FD = dyn_cast<FunctionDecl>(&Node))
31+
return FD->isInlineSpecified();
32+
if (const auto *VD = dyn_cast<VarDecl>(&Node))
33+
return VD->isInlineSpecified();
34+
llvm_unreachable("Not a valid polymorphic type");
35+
}
36+
37+
2738
static std::optional<SourceLocation>
2839
getInlineTokenLocation(SourceRange RangeLocation, const SourceManager &Sources,
2940
const LangOptions &LangOpts) {
30-
SourceLocation CurrentLocation = RangeLocation.getBegin();
31-
Token CurrentToken;
32-
while (!Lexer::getRawToken(CurrentLocation, CurrentToken, Sources, LangOpts,
33-
true) &&
34-
CurrentLocation < RangeLocation.getEnd() &&
35-
CurrentToken.isNot(tok::eof)) {
36-
if (CurrentToken.is(tok::raw_identifier)) {
37-
if (CurrentToken.getRawIdentifier() == "inline") {
38-
return CurrentToken.getLocation();
39-
}
40-
}
41-
CurrentLocation = CurrentToken.getEndLoc();
41+
Token FirstToken;
42+
Lexer::getRawToken(RangeLocation.getBegin(), FirstToken, Sources, LangOpts,
43+
true);
44+
std::optional<Token> CurrentToken = FirstToken;
45+
while (CurrentToken && CurrentToken->getLocation() < RangeLocation.getEnd() &&
46+
CurrentToken->isNot(tok::eof)) {
47+
if (CurrentToken->is(tok::raw_identifier) &&
48+
CurrentToken->getRawIdentifier() == "inline")
49+
return CurrentToken->getLocation();
50+
51+
CurrentToken = Lexer::findNextToken(CurrentToken->getLocation(), Sources, LangOpts);
4252
}
4353
return std::nullopt;
4454
}
4555

4656
void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) {
4757
Finder->addMatcher(
48-
functionDecl(unless(isExpansionInSystemHeader()), unless(isImplicit()),
49-
unless(hasAncestor(lambdaExpr())), isInline(),
58+
functionDecl(unless(isExpansionInSystemHeader()),
59+
unless(hasAncestor(lambdaExpr())), isInlineSpecified(),
5060
anyOf(isConstexpr(), isDeleted(),
5161
allOf(isDefinition(), hasAncestor(recordDecl()),
5262
unless(hasAncestor(cxxConstructorDecl())))))
5363
.bind("fun_decl"),
5464
this);
5565

5666
Finder->addMatcher(
57-
varDecl(isInline(), unless(isImplicit()),
67+
varDecl(isInlineSpecified(),
5868
anyOf(allOf(isConstexpr(), unless(isStaticStorageClass())),
5969
hasAncestor(recordDecl())))
6070
.bind("var_decl"),
6171
this);
6272

6373
Finder->addMatcher(
64-
functionTemplateDecl(has(functionDecl(isInline()))).bind("templ_decl"),
74+
functionTemplateDecl(has(functionDecl(isInlineSpecified()))).bind("templ_decl"),
6575
this);
6676
}
6777

6878
template <typename T>
6979
void RedundantInlineSpecifierCheck::handleMatchedDecl(
7080
const T *MatchedDecl, const SourceManager &Sources,
71-
const MatchFinder::MatchResult &Result, const char *Message) {
72-
if (auto Loc = getInlineTokenLocation(MatchedDecl->getSourceRange(), Sources,
81+
const MatchFinder::MatchResult &Result, StringRef Message) {
82+
if (std::optional<SourceLocation> Loc = getInlineTokenLocation(MatchedDecl->getSourceRange(), Sources,
7383
Result.Context->getLangOpts()))
7484
diag(*Loc, Message) << MatchedDecl << FixItHint::CreateRemoval(*Loc);
7585
}
@@ -82,17 +92,17 @@ void RedundantInlineSpecifierCheck::check(
8292
Result.Nodes.getNodeAs<FunctionDecl>("fun_decl")) {
8393
handleMatchedDecl(
8494
MatchedDecl, Sources, Result,
85-
"Function %0 has inline specifier but is implicitly inlined");
95+
"function %0 has inline specifier but is implicitly inlined");
8696
} else if (const auto *MatchedDecl =
8797
Result.Nodes.getNodeAs<VarDecl>("var_decl")) {
8898
handleMatchedDecl(
8999
MatchedDecl, Sources, Result,
90-
"Variable %0 has inline specifier but is implicitly inlined");
100+
"variable %0 has inline specifier but is implicitly inlined");
91101
} else if (const auto *MatchedDecl =
92102
Result.Nodes.getNodeAs<FunctionTemplateDecl>("templ_decl")) {
93103
handleMatchedDecl(
94104
MatchedDecl, Sources, Result,
95-
"Function %0 has inline specifier but is implicitly inlined");
105+
"function %0 has inline specifier but is implicitly inlined");
96106
}
97107
}
98108

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
namespace clang::tidy::readability {
1515

16-
/// Finds redundant `inline` specifiers in function and variable declarations.
16+
/// Detects redundant ``inline`` specifiers on function and variable declarations.
1717
///
1818
/// For the user-facing documentation see:
1919
/// http://clang.llvm.org/extra/clang-tidy/checks/readability/readability-redundant-inline-specifier.html
@@ -23,12 +23,15 @@ class RedundantInlineSpecifierCheck : public ClangTidyCheck {
2323
: ClangTidyCheck(Name, Context) {}
2424
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
2525
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
26+
std::optional<TraversalKind> getCheckTraversalKind() const override {
27+
return TK_IgnoreUnlessSpelledInSource;
28+
}
2629

2730
private:
2831
template <typename T>
2932
void handleMatchedDecl(const T *MatchedDecl, const SourceManager &Sources,
3033
const ast_matchers::MatchFinder::MatchResult &Result,
31-
const char *Message);
34+
StringRef Message);
3235
};
3336

3437
} // namespace clang::tidy::readability

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

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
readability-redundant-inline-specifier
44
======================================
55

6-
Checks for instances of the `inline` keyword in code where it is redundant
6+
Checks for instances of the ``inline`` keyword in code where it is redundant
77
and recommends its removal.
88

99
Examples:
@@ -12,7 +12,7 @@ Examples:
1212

1313
constexpr inline void f() {}
1414

15-
In the example abvove the keyword `inline` is redundant since constexpr
15+
In the example above the keyword ``inline`` is redundant since constexpr
1616
functions are implicitly inlined
1717

1818
.. code-block:: c++
@@ -21,15 +21,5 @@ functions are implicitly inlined
2121
inline void myMethod() {}
2222
};
2323

24-
In the example above the keyword `inline` is redundant since member functions
24+
In the example above the keyword ``inline`` is redundant since member functions
2525
defined entirely inside a class/struct/union definition are implicitly inlined.
26-
27-
The token `inline` is considered redundant in the following cases:
28-
29-
- When it is used in a function definition that is constexpr.
30-
- When it is used in a member function definition that is defined entirely
31-
inside a class/struct/union definition.
32-
- When it is used on a deleted function.
33-
- When it is used on a template declaration.
34-
- When it is used on a member variable that is constexpr and static.
35-

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

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,73 +1,73 @@
11
// RUN: %check_clang_tidy %s readability-redundant-inline-specifier %t
22

33
template <typename T> inline T f()
4-
// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: Function 'f' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
4+
// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: function 'f' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
55
// CHECK-FIXES: template <typename T> T f()
66
{
77
return T{};
88
}
99

1010
template <> inline double f<double>() = delete;
11-
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: Function 'f<double>' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
11+
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function 'f<double>' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
1212
// CHECK-FIXES: template <> double f<double>() = delete;
1313

1414
inline int g(float a)
15-
// CHECK-MESSAGES-NOT: :[[@LINE-1]]:1: warning: Function 'g' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
15+
// CHECK-MESSAGES-NOT: :[[@LINE-1]]:1: warning: function 'g' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
1616
{
1717
return static_cast<int>(a - 5.F);
1818
}
1919

2020
inline int g(double) = delete;
21-
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: Function 'g' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
21+
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: function 'g' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
2222
// CHECK-FIXES: int g(double) = delete;
2323

2424
class C
2525
{
2626
public:
2727
inline C& operator=(const C&) = delete;
28-
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: Function 'operator=' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
28+
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'operator=' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
2929
// CHECK-FIXES: C& operator=(const C&) = delete;
3030

3131
constexpr inline C& operator=(int a);
32-
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: Function 'operator=' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
32+
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: function 'operator=' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
3333
// CHECK-FIXES: constexpr C& operator=(int a);
3434

3535
inline C() {}
36-
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: Function 'C' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
36+
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'C' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
3737
// CHECK-FIXES: C() {}
3838

3939
constexpr inline C(int);
40-
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: Function 'C' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
40+
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: function 'C' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
4141
// CHECK-FIXES: constexpr C(int);
4242

4343
inline int Get42() const { return 42; }
44-
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: Function 'Get42' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
44+
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'Get42' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
4545
// CHECK-FIXES: int Get42() const { return 42; }
4646

4747
static inline constexpr int C_STATIC = 42;
48-
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: Variable 'C_STATIC' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
48+
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'C_STATIC' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
4949
// CHECK-FIXES: static constexpr int C_STATIC = 42;
5050

5151
static constexpr int C_STATIC_2 = 42;
52-
// CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: Variable 'C_STATIC_2' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
52+
// CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: variable 'C_STATIC_2' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
5353
};
5454

5555
constexpr inline int Get42() { return 42; }
56-
// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: Function 'Get42' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
56+
// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: function 'Get42' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
5757
// CHECK-FIXES: constexpr int Get42() { return 42; }
5858

5959

6060
static constexpr inline int NAMESPACE_STATIC = 42;
61-
// CHECK-MESSAGES-NOT: :[[@LINE-1]]:18: warning: Variable 'NAMESPACE_STATIC' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
61+
// CHECK-MESSAGES-NOT: :[[@LINE-1]]:18: warning: variable 'NAMESPACE_STATIC' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
6262

6363
inline static int fn0(int i)
64-
// CHECK-MESSAGES-NOT: :[[@LINE-1]]:1: warning: Function 'fn0' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
64+
// CHECK-MESSAGES-NOT: :[[@LINE-1]]:1: warning: function 'fn0' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
6565
{
6666
return i - 1;
6767
}
6868

6969
static constexpr inline int fn1(int i)
70-
// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: Function 'fn1' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
70+
// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: function 'fn1' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
7171
// CHECK-FIXES: static constexpr int fn1(int i)
7272
{
7373
return i - 1;
@@ -76,13 +76,13 @@ static constexpr inline int fn1(int i)
7676
namespace
7777
{
7878
inline int fn2(int i)
79-
// CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: Function 'fn2' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
79+
// CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: function 'fn2' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
8080
{
8181
return i - 1;
8282
}
8383

8484
inline constexpr int fn3(int i)
85-
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: Function 'fn3' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
85+
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'fn3' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
8686
// CHECK-FIXES: constexpr int fn3(int i)
8787
{
8888
return i - 1;
@@ -92,19 +92,19 @@ namespace
9292
namespace ns
9393
{
9494
inline int fn4(int i)
95-
// CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: Function 'fn4' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
95+
// CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: function 'fn4' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
9696
{
9797
return i - 1;
9898
}
9999

100100
inline constexpr int fn5(int i)
101-
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: Function 'fn5' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
101+
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'fn5' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
102102
// CHECK-FIXES: constexpr int fn5(int i)
103103
{
104104
return i - 1;
105105
}
106106
}
107107

108108
auto fn6 = [](){};
109-
//CHECK-MESSAGES-NOT: :[[@LINE-1]]:1: warning: Function 'operator()' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
109+
//CHECK-MESSAGES-NOT: :[[@LINE-1]]:1: warning: function 'operator()' has inline specifier but is implicitly inlined [readability-redundant-inline-specifier]
110110

‎clang/include/clang/ASTMatchers/ASTMatchers.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7889,7 +7889,7 @@ AST_POLYMORPHIC_MATCHER(isInline, AST_POLYMORPHIC_SUPPORTED_TYPES(NamespaceDecl,
78897889
if (const auto *NSD = dyn_cast<NamespaceDecl>(&Node))
78907890
return NSD->isInline();
78917891
if (const auto *VD = dyn_cast<VarDecl>(&Node))
7892-
return VD->isInlineSpecified();
7892+
return VD->isInline();
78937893
llvm_unreachable("Not a valid polymorphic type");
78947894
}
78957895

0 commit comments

Comments
 (0)