Skip to content

Commit 795b3ae

Browse files
committed
[clang-tidy] insert static keyword in correct position for misc-use-internal-linkage
Fixes: #108760
1 parent 095b41c commit 795b3ae

File tree

5 files changed

+72
-3
lines changed

5 files changed

+72
-3
lines changed

clang-tools-extra/clang-tidy/misc/UseInternalLinkageCheck.cpp

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,15 @@
88

99
#include "UseInternalLinkageCheck.h"
1010
#include "../utils/FileExtensionsUtils.h"
11+
#include "../utils/LexerUtils.h"
1112
#include "clang/AST/Decl.h"
1213
#include "clang/ASTMatchers/ASTMatchFinder.h"
1314
#include "clang/ASTMatchers/ASTMatchers.h"
1415
#include "clang/ASTMatchers/ASTMatchersMacros.h"
1516
#include "clang/Basic/SourceLocation.h"
1617
#include "clang/Basic/Specifiers.h"
18+
#include "clang/Basic/TokenKinds.h"
19+
#include "clang/Lex/Token.h"
1720
#include "llvm/ADT/STLExtras.h"
1821

1922
using namespace clang::ast_matchers;
@@ -110,10 +113,36 @@ static constexpr StringRef Message =
110113
"%0 %1 can be made static or moved into an anonymous namespace "
111114
"to enforce internal linkage";
112115

116+
static SourceLocation getQualifiedTypeStartLoc(SourceLocation L,
117+
const SourceManager &SM,
118+
const ASTContext &Context) {
119+
const SourceLocation StartOfFile = SM.getLocForStartOfFile(SM.getFileID(L));
120+
if (L.isInvalid() || L.isMacroID())
121+
return L;
122+
bool HasChanged = true;
123+
while (HasChanged) {
124+
if (L == StartOfFile)
125+
return L;
126+
auto [Tok, Loc] =
127+
utils::lexer::getPreviousTokenAndStart(L, SM, Context.getLangOpts());
128+
if (Tok.is(tok::raw_identifier)) {
129+
IdentifierInfo &Info = Context.Idents.get(
130+
StringRef(SM.getCharacterData(Tok.getLocation()), Tok.getLength()));
131+
Tok.setIdentifierInfo(&Info);
132+
Tok.setKind(Info.getTokenID());
133+
}
134+
HasChanged = Tok.isOneOf(tok::kw_const, tok::kw_volatile);
135+
if (HasChanged)
136+
L = Loc;
137+
}
138+
return L;
139+
}
140+
113141
void UseInternalLinkageCheck::check(const MatchFinder::MatchResult &Result) {
114142
if (const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>("fn")) {
115143
DiagnosticBuilder DB = diag(FD->getLocation(), Message) << "function" << FD;
116-
SourceLocation FixLoc = FD->getTypeSpecStartLoc();
144+
const SourceLocation FixLoc = getQualifiedTypeStartLoc(
145+
FD->getTypeSpecStartLoc(), *Result.SourceManager, *Result.Context);
117146
if (FixLoc.isInvalid() || FixLoc.isMacroID())
118147
return;
119148
if (FixMode == FixModeKind::UseStatic)
@@ -128,7 +157,8 @@ void UseInternalLinkageCheck::check(const MatchFinder::MatchResult &Result) {
128157
return;
129158

130159
DiagnosticBuilder DB = diag(VD->getLocation(), Message) << "variable" << VD;
131-
SourceLocation FixLoc = VD->getTypeSpecStartLoc();
160+
const SourceLocation FixLoc = getQualifiedTypeStartLoc(
161+
VD->getTypeSpecStartLoc(), *Result.SourceManager, *Result.Context);
132162
if (FixLoc.isInvalid() || FixLoc.isMacroID())
133163
return;
134164
if (FixMode == FixModeKind::UseStatic)

clang-tools-extra/clang-tidy/utils/LexerUtils.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,15 @@ getPreviousTokenAndStart(SourceLocation Location, const SourceManager &SM,
2424
if (Location.isInvalid())
2525
return {Token, Location};
2626

27-
auto StartOfFile = SM.getLocForStartOfFile(SM.getFileID(Location));
27+
const auto StartOfFile = SM.getLocForStartOfFile(SM.getFileID(Location));
2828
while (Location != StartOfFile) {
2929
Location = Lexer::GetBeginningOfToken(Location, SM, LangOpts);
3030
if (!Lexer::getRawToken(Location, Token, SM, LangOpts) &&
3131
(!SkipComments || !Token.is(tok::comment))) {
3232
break;
3333
}
34+
if (Location == StartOfFile)
35+
return {Token, Location};
3436
Location = Location.getLocWithOffset(-1);
3537
}
3638
return {Token, Location};

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,10 @@ Changes in existing checks
128128
<clang-tidy/checks/misc/unconventional-assign-operator>` check to avoid
129129
false positive for C++23 deducing this.
130130

131+
- Improved :doc:`misc-use-internal-linkage
132+
<clang-tidy/checks/misc/use-internal-linkage>` check to insert ``static`` keyword
133+
before type qualifier such as ``const``, ``volatile``.
134+
131135
- Improved :doc:`modernize-use-std-print
132136
<clang-tidy/checks/modernize/use-std-print>` check to support replacing
133137
member function calls too.

clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-func.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,27 @@ void func_cpp_inc();
1717
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_cpp_inc'
1818
// CHECK-FIXES: static void func_cpp_inc();
1919

20+
int* func_cpp_inc_return_ptr();
21+
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_cpp_inc_return_ptr'
22+
// CHECK-FIXES: static int* func_cpp_inc_return_ptr();
23+
24+
const int* func_cpp_inc_return_const_ptr();
25+
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: function 'func_cpp_inc_return_const_ptr'
26+
// CHECK-FIXES: static const int* func_cpp_inc_return_const_ptr();
27+
28+
int const* func_cpp_inc_return_ptr_const();
29+
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: function 'func_cpp_inc_return_ptr_const'
30+
// CHECK-FIXES: static int const* func_cpp_inc_return_ptr_const();
31+
32+
int * const func_cpp_inc_return_const();
33+
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function 'func_cpp_inc_return_const'
34+
// CHECK-FIXES: static int * const func_cpp_inc_return_const();
35+
36+
volatile const int* func_cpp_inc_return_volatile_const_ptr();
37+
// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: function 'func_cpp_inc_return_volatile_const_ptr'
38+
// CHECK-FIXES: static volatile const int* func_cpp_inc_return_volatile_const_ptr();
39+
40+
2041
#include "func_cpp.inc"
2142

2243
void func_h_inc();

clang-tools-extra/test/clang-tidy/checkers/misc/use-internal-linkage-var.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,18 @@ T global_template;
1313
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: variable 'global_template'
1414
// CHECK-FIXES: static T global_template;
1515

16+
int const* ptr_const_star;
17+
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'ptr_const_star'
18+
// CHECK-FIXES: static int const* ptr_const_star;
19+
20+
const int* const_ptr_star;
21+
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'const_ptr_star'
22+
// CHECK-FIXES: static const int* const_ptr_star;
23+
24+
const volatile int* const_volatile_ptr_star;
25+
// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: variable 'const_volatile_ptr_star'
26+
// CHECK-FIXES: static const volatile int* const_volatile_ptr_star;
27+
1628
int gloabl_header;
1729

1830
extern int global_extern;

0 commit comments

Comments
 (0)