Skip to content

Commit fbd86d0

Browse files
authored
[clang-reorder-fields] Reorder leading comments (#123740)
Similarly to #122918, leading comments are currently not being moved. ``` struct Foo { // This one is the cool field. int a; int b; }; ``` becomes: ``` struct Foo { // This one is the cool field. int b; int a; }; ``` but should be: ``` struct Foo { int b; // This one is the cool field. int a; }; ```
1 parent c6c6475 commit fbd86d0

File tree

6 files changed

+101
-21
lines changed

6 files changed

+101
-21
lines changed

clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,29 @@ findMembersUsedInInitExpr(const CXXCtorInitializer *Initializer,
118118
return Results;
119119
}
120120

121+
/// Returns the start of the leading comments before `Loc`.
122+
static SourceLocation getStartOfLeadingComment(SourceLocation Loc,
123+
const SourceManager &SM,
124+
const LangOptions &LangOpts) {
125+
// We consider any leading comment token that is on the same line or
126+
// indented similarly to the first comment to be part of the leading comment.
127+
const unsigned Line = SM.getPresumedLineNumber(Loc);
128+
const unsigned Column = SM.getPresumedColumnNumber(Loc);
129+
std::optional<Token> Tok =
130+
Lexer::findPreviousToken(Loc, SM, LangOpts, /*IncludeComments=*/true);
131+
while (Tok && Tok->is(tok::comment)) {
132+
const SourceLocation CommentLoc =
133+
Lexer::GetBeginningOfToken(Tok->getLocation(), SM, LangOpts);
134+
if (SM.getPresumedLineNumber(CommentLoc) != Line &&
135+
SM.getPresumedColumnNumber(CommentLoc) != Column) {
136+
break;
137+
}
138+
Loc = CommentLoc;
139+
Tok = Lexer::findPreviousToken(Loc, SM, LangOpts, /*IncludeComments=*/true);
140+
}
141+
return Loc;
142+
}
143+
121144
/// Returns the end of the trailing comments after `Loc`.
122145
static SourceLocation getEndOfTrailingComment(SourceLocation Loc,
123146
const SourceManager &SM,
@@ -159,6 +182,7 @@ static SourceRange getFullFieldSourceRange(const FieldDecl &Field,
159182
if (CurrentToken->is(tok::semi))
160183
break;
161184
}
185+
Begin = getStartOfLeadingComment(Begin, SM, LangOpts);
162186
End = getEndOfTrailingComment(End, SM, LangOpts);
163187
return SourceRange(Begin, End);
164188
}

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

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -17,25 +17,16 @@ namespace clang::tidy::utils::lexer {
1717
std::pair<Token, SourceLocation>
1818
getPreviousTokenAndStart(SourceLocation Location, const SourceManager &SM,
1919
const LangOptions &LangOpts, bool SkipComments) {
20-
Token Token;
21-
Token.setKind(tok::unknown);
20+
const std::optional<Token> Tok =
21+
Lexer::findPreviousToken(Location, SM, LangOpts, !SkipComments);
2222

23-
Location = Location.getLocWithOffset(-1);
24-
if (Location.isInvalid())
25-
return {Token, Location};
26-
27-
const auto StartOfFile = SM.getLocForStartOfFile(SM.getFileID(Location));
28-
while (Location != StartOfFile) {
29-
Location = Lexer::GetBeginningOfToken(Location, SM, LangOpts);
30-
if (!Lexer::getRawToken(Location, Token, SM, LangOpts) &&
31-
(!SkipComments || !Token.is(tok::comment))) {
32-
break;
33-
}
34-
if (Location == StartOfFile)
35-
return {Token, Location};
36-
Location = Location.getLocWithOffset(-1);
23+
if (Tok.has_value()) {
24+
return {*Tok, Lexer::GetBeginningOfToken(Tok->getLocation(), SM, LangOpts)};
3725
}
38-
return {Token, Location};
26+
27+
Token Token;
28+
Token.setKind(tok::unknown);
29+
return {Token, SourceLocation()};
3930
}
4031

4132
Token getPreviousToken(SourceLocation Location, const SourceManager &SM,

clang-tools-extra/test/clang-reorder-fields/Comments.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: clang-reorder-fields -record-name Foo -fields-order e1,e3,e2,a,c,b %s -- | FileCheck %s
1+
// RUN: clang-reorder-fields -record-name Foo -fields-order c,e1,e3,e2,a,b %s -- | FileCheck %s
22

33
class Foo {
44
int a; // Trailing comment for a.
@@ -12,12 +12,15 @@ class Foo {
1212
int e3 /*c-like*/;
1313
};
1414

15-
// CHECK: /*c-like*/ int e1;
15+
// Note: the position of the empty line is somewhat arbitrary.
16+
17+
// CHECK: // Prefix comments for c.
18+
// CHECK-NEXT: int c;
19+
// CHECK-NEXT: /*c-like*/ int e1;
1620
// CHECK-NEXT: int e3 /*c-like*/;
21+
// CHECK-EMPTY:
1722
// CHECK-NEXT: int /*c-like*/ e2;
1823
// CHECK-NEXT: int a; // Trailing comment for a.
19-
// CHECK-NEXT: // Prefix comments for c.
20-
// CHECK-NEXT: int c;
2124
// CHECK-NEXT: int b; // Multiline
2225
// CHECK-NEXT: // trailing for b.
2326

clang/include/clang/Lex/Lexer.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -557,6 +557,12 @@ class Lexer : public PreprocessorLexer {
557557
const LangOptions &LangOpts,
558558
bool IncludeComments = false);
559559

560+
/// Finds the token that comes before the given location.
561+
static std::optional<Token> findPreviousToken(SourceLocation Loc,
562+
const SourceManager &SM,
563+
const LangOptions &LangOpts,
564+
bool IncludeComments);
565+
560566
/// Checks that the given token is the first token that occurs after
561567
/// the given location (this excludes comments and whitespace). Returns the
562568
/// location immediately after the specified token. If the token is not found

clang/lib/Lex/Lexer.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1352,6 +1352,27 @@ std::optional<Token> Lexer::findNextToken(SourceLocation Loc,
13521352
return Tok;
13531353
}
13541354

1355+
std::optional<Token> Lexer::findPreviousToken(SourceLocation Loc,
1356+
const SourceManager &SM,
1357+
const LangOptions &LangOpts,
1358+
bool IncludeComments) {
1359+
const auto StartOfFile = SM.getLocForStartOfFile(SM.getFileID(Loc));
1360+
while (Loc != StartOfFile) {
1361+
Loc = Loc.getLocWithOffset(-1);
1362+
if (Loc.isInvalid())
1363+
return std::nullopt;
1364+
1365+
Loc = GetBeginningOfToken(Loc, SM, LangOpts);
1366+
Token Tok;
1367+
if (getRawToken(Loc, Tok, SM, LangOpts))
1368+
continue; // Not a token, go to prev location.
1369+
if (!Tok.is(tok::comment) || IncludeComments) {
1370+
return Tok;
1371+
}
1372+
}
1373+
return std::nullopt;
1374+
}
1375+
13551376
/// Checks that the given token is the first token that occurs after the
13561377
/// given location (this excludes comments and whitespace). Returns the location
13571378
/// immediately after the specified token. If the token is not found or the

clang/unittests/Lex/LexerTest.cpp

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -640,6 +640,41 @@ TEST_F(LexerTest, FindNextTokenIncludingComments) {
640640
"=", "abcd", ";"));
641641
}
642642

643+
TEST_F(LexerTest, FindPreviousToken) {
644+
Lex("int abcd = 0;\n"
645+
"// A comment.\n"
646+
"int xyz = abcd;\n");
647+
std::vector<std::string> GeneratedByPrevToken;
648+
SourceLocation Loc = SourceMgr.getLocForEndOfFile(SourceMgr.getMainFileID());
649+
while (true) {
650+
auto T = Lexer::findPreviousToken(Loc, SourceMgr, LangOpts, false);
651+
if (!T.has_value())
652+
break;
653+
GeneratedByPrevToken.push_back(getSourceText(*T, *T));
654+
Loc = Lexer::GetBeginningOfToken(T->getLocation(), SourceMgr, LangOpts);
655+
}
656+
EXPECT_THAT(GeneratedByPrevToken, ElementsAre(";", "abcd", "=", "xyz", "int",
657+
";", "0", "=", "abcd", "int"));
658+
}
659+
660+
TEST_F(LexerTest, FindPreviousTokenIncludingComments) {
661+
Lex("int abcd = 0;\n"
662+
"// A comment.\n"
663+
"int xyz = abcd;\n");
664+
std::vector<std::string> GeneratedByPrevToken;
665+
SourceLocation Loc = SourceMgr.getLocForEndOfFile(SourceMgr.getMainFileID());
666+
while (true) {
667+
auto T = Lexer::findPreviousToken(Loc, SourceMgr, LangOpts, true);
668+
if (!T.has_value())
669+
break;
670+
GeneratedByPrevToken.push_back(getSourceText(*T, *T));
671+
Loc = Lexer::GetBeginningOfToken(T->getLocation(), SourceMgr, LangOpts);
672+
}
673+
EXPECT_THAT(GeneratedByPrevToken,
674+
ElementsAre(";", "abcd", "=", "xyz", "int", "// A comment.", ";",
675+
"0", "=", "abcd", "int"));
676+
}
677+
643678
TEST_F(LexerTest, CreatedFIDCountForPredefinedBuffer) {
644679
TrivialModuleLoader ModLoader;
645680
auto PP = CreatePP("", ModLoader);

0 commit comments

Comments
 (0)