[clang] [clang-tools-extra] [clang][refactor] Refactor `findNextTokenIncludingComments` (PR #123060)
Clement Courbet via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 16 00:50:05 PST 2025
https://github.com/legrosbuffle updated https://github.com/llvm/llvm-project/pull/123060
>From 005a730b72be1305c67f886d9a473273d7318d99 Mon Sep 17 00:00:00 2001
From: Clement Courbet <courbet at google.com>
Date: Wed, 15 Jan 2025 12:19:58 +0000
Subject: [PATCH 1/4] [clang][refactor] Refactor
`findNextTokenIncludingComments`
We have two copies of the same code in clang-tidy and
clang-reorder-fields, and those are extremenly similar to
`Lexer::findNextToken`, so just add an extra agument to the latter.
---
.../ReorderFieldsAction.cpp | 34 ++-----------------
.../modernize/ConcatNestedNamespacesCheck.cpp | 2 +-
.../clang-tidy/modernize/UseOverrideCheck.cpp | 5 ++-
.../clang-tidy/utils/LexerUtils.cpp | 23 -------------
.../clang-tidy/utils/LexerUtils.h | 4 ++-
clang/include/clang/Lex/Lexer.h | 3 +-
clang/lib/Lex/Lexer.cpp | 4 ++-
7 files changed, 14 insertions(+), 61 deletions(-)
diff --git a/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp b/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
index 80ee31368fe9a5..40c96f92254e42 100644
--- a/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
+++ b/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
@@ -118,35 +118,6 @@ findMembersUsedInInitExpr(const CXXCtorInitializer *Initializer,
return Results;
}
-/// Returns the next token after `Loc` (including comment tokens).
-static std::optional<Token> getTokenAfter(SourceLocation Loc,
- const SourceManager &SM,
- const LangOptions &LangOpts) {
- if (Loc.isMacroID()) {
- return std::nullopt;
- }
- Loc = Lexer::getLocForEndOfToken(Loc, 0, SM, LangOpts);
-
- // Break down the source location.
- std::pair<FileID, unsigned> LocInfo = SM.getDecomposedLoc(Loc);
-
- // Try to load the file buffer.
- bool InvalidTemp = false;
- StringRef File = SM.getBufferData(LocInfo.first, &InvalidTemp);
- if (InvalidTemp)
- return std::nullopt;
-
- const char *TokenBegin = File.data() + LocInfo.second;
-
- Lexer lexer(SM.getLocForStartOfFile(LocInfo.first), LangOpts, File.begin(),
- TokenBegin, File.end());
- lexer.SetCommentRetentionState(true);
- // Find the token.
- Token Tok;
- lexer.LexFromRawLexer(Tok);
- return Tok;
-}
-
/// Returns the end of the trailing comments after `Loc`.
static SourceLocation getEndOfTrailingComment(SourceLocation Loc,
const SourceManager &SM,
@@ -154,11 +125,12 @@ static SourceLocation getEndOfTrailingComment(SourceLocation Loc,
// We consider any following comment token that is indented more than the
// first comment to be part of the trailing comment.
const unsigned Column = SM.getPresumedColumnNumber(Loc);
- std::optional<Token> Tok = getTokenAfter(Loc, SM, LangOpts);
+ std::optional<Token> Tok =
+ Lexer::findNextToken(Loc, SM, LangOpts, /*IncludeComments*/ true);
while (Tok && Tok->is(tok::comment) &&
SM.getPresumedColumnNumber(Tok->getLocation()) > Column) {
Loc = Tok->getEndLoc();
- Tok = getTokenAfter(Loc, SM, LangOpts);
+ Tok = Lexer::findNextToken(Loc, SM, LangOpts, /*IncludeComments*/ true);
}
return Loc;
}
diff --git a/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp b/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
index d24b613015d8ee..b4f54d02fc3362 100644
--- a/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
@@ -59,7 +59,7 @@ SourceRange NS::getNamespaceBackRange(const SourceManager &SM,
// Back from '}' to conditional '// namespace xxx'
SourceLocation Loc = front()->getRBraceLoc();
std::optional<Token> Tok =
- utils::lexer::findNextTokenIncludingComments(Loc, SM, LangOpts);
+ Lexer::findNextToken(Loc, SM, LangOpts, /*IncludeComments*/ true);
if (!Tok)
return getDefaultNamespaceBackRange();
if (Tok->getKind() != tok::TokenKind::comment)
diff --git a/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
index fd5bd9f0b181b1..6191ebfbfb01f0 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
@@ -229,9 +229,8 @@ void UseOverrideCheck::check(const MatchFinder::MatchResult &Result) {
if (HasVirtual) {
for (Token Tok : Tokens) {
if (Tok.is(tok::kw_virtual)) {
- std::optional<Token> NextToken =
- utils::lexer::findNextTokenIncludingComments(
- Tok.getEndLoc(), Sources, getLangOpts());
+ std::optional<Token> NextToken = Lexer::findNextToken(
+ Tok.getEndLoc(), Sources, getLangOpts(), /*IncludeComments*/ true);
if (NextToken.has_value()) {
Diag << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
Tok.getLocation(), NextToken->getLocation()));
diff --git a/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp b/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
index 92c3e0ed7894e1..50da196315d3b3 100644
--- a/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
+++ b/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
@@ -86,29 +86,6 @@ SourceLocation findNextTerminator(SourceLocation Start, const SourceManager &SM,
return findNextAnyTokenKind(Start, SM, LangOpts, tok::comma, tok::semi);
}
-std::optional<Token>
-findNextTokenIncludingComments(SourceLocation Start, const SourceManager &SM,
- const LangOptions &LangOpts) {
- // `Lexer::findNextToken` will ignore comment
- if (Start.isMacroID())
- return std::nullopt;
- Start = Lexer::getLocForEndOfToken(Start, 0, SM, LangOpts);
- // Break down the source location.
- std::pair<FileID, unsigned> LocInfo = SM.getDecomposedLoc(Start);
- bool InvalidTemp = false;
- StringRef File = SM.getBufferData(LocInfo.first, &InvalidTemp);
- if (InvalidTemp)
- return std::nullopt;
- // Lex from the start of the given location.
- Lexer L(SM.getLocForStartOfFile(LocInfo.first), LangOpts, File.begin(),
- File.data() + LocInfo.second, File.end());
- L.SetCommentRetentionState(true);
- // Find the token.
- Token Tok;
- L.LexFromRawLexer(Tok);
- return Tok;
-}
-
std::optional<Token>
findNextTokenSkippingComments(SourceLocation Start, const SourceManager &SM,
const LangOptions &LangOpts) {
diff --git a/clang-tools-extra/clang-tidy/utils/LexerUtils.h b/clang-tools-extra/clang-tidy/utils/LexerUtils.h
index ea9bd512b68b8f..c0993e8872b1e8 100644
--- a/clang-tools-extra/clang-tidy/utils/LexerUtils.h
+++ b/clang-tools-extra/clang-tidy/utils/LexerUtils.h
@@ -91,7 +91,9 @@ SourceLocation findNextAnyTokenKind(SourceLocation Start,
std::optional<Token>
findNextTokenIncludingComments(SourceLocation Start, const SourceManager &SM,
- const LangOptions &LangOpts);
+ const LangOptions &LangOpts) {
+ return Lexer::findNextToken(Start, SM, LangOpts, true);
+}
// Finds next token that's not a comment.
std::optional<Token> findNextTokenSkippingComments(SourceLocation Start,
diff --git a/clang/include/clang/Lex/Lexer.h b/clang/include/clang/Lex/Lexer.h
index b6ecc7e5ded9e2..82a041ea3f848a 100644
--- a/clang/include/clang/Lex/Lexer.h
+++ b/clang/include/clang/Lex/Lexer.h
@@ -554,7 +554,8 @@ class Lexer : public PreprocessorLexer {
/// Returns the next token, or std::nullopt if the location is inside a macro.
static std::optional<Token> findNextToken(SourceLocation Loc,
const SourceManager &SM,
- const LangOptions &LangOpts);
+ const LangOptions &LangOpts,
+ bool IncludeComments = false);
/// Checks that the given token is the first token that occurs after
/// the given location (this excludes comments and whitespace). Returns the
diff --git a/clang/lib/Lex/Lexer.cpp b/clang/lib/Lex/Lexer.cpp
index 72364500a48f9f..115b6c1606a022 100644
--- a/clang/lib/Lex/Lexer.cpp
+++ b/clang/lib/Lex/Lexer.cpp
@@ -1323,7 +1323,8 @@ const char *Lexer::SkipEscapedNewLines(const char *P) {
std::optional<Token> Lexer::findNextToken(SourceLocation Loc,
const SourceManager &SM,
- const LangOptions &LangOpts) {
+ const LangOptions &LangOpts,
+ bool IncludeComments) {
if (Loc.isMacroID()) {
if (!Lexer::isAtEndOfMacroExpansion(Loc, SM, LangOpts, &Loc))
return std::nullopt;
@@ -1344,6 +1345,7 @@ std::optional<Token> Lexer::findNextToken(SourceLocation Loc,
// Lex from the start of the given location.
Lexer lexer(SM.getLocForStartOfFile(LocInfo.first), LangOpts, File.begin(),
TokenBegin, File.end());
+ lexer.SetCommentRetentionState(IncludeComments);
// Find the token.
Token Tok;
lexer.LexFromRawLexer(Tok);
>From c1f46033999a9618e02e4bd7acbfde8a221b5653 Mon Sep 17 00:00:00 2001
From: Clement Courbet <courbet at google.com>
Date: Wed, 15 Jan 2025 15:56:06 +0000
Subject: [PATCH 2/4] address review comments.
---
.../clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp | 2 +-
clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp | 5 +++--
clang-tools-extra/clang-tidy/utils/LexerUtils.h | 2 +-
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp b/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
index b4f54d02fc3362..d24b613015d8ee 100644
--- a/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
@@ -59,7 +59,7 @@ SourceRange NS::getNamespaceBackRange(const SourceManager &SM,
// Back from '}' to conditional '// namespace xxx'
SourceLocation Loc = front()->getRBraceLoc();
std::optional<Token> Tok =
- Lexer::findNextToken(Loc, SM, LangOpts, /*IncludeComments*/ true);
+ utils::lexer::findNextTokenIncludingComments(Loc, SM, LangOpts);
if (!Tok)
return getDefaultNamespaceBackRange();
if (Tok->getKind() != tok::TokenKind::comment)
diff --git a/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
index 6191ebfbfb01f0..fd5bd9f0b181b1 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
@@ -229,8 +229,9 @@ void UseOverrideCheck::check(const MatchFinder::MatchResult &Result) {
if (HasVirtual) {
for (Token Tok : Tokens) {
if (Tok.is(tok::kw_virtual)) {
- std::optional<Token> NextToken = Lexer::findNextToken(
- Tok.getEndLoc(), Sources, getLangOpts(), /*IncludeComments*/ true);
+ std::optional<Token> NextToken =
+ utils::lexer::findNextTokenIncludingComments(
+ Tok.getEndLoc(), Sources, getLangOpts());
if (NextToken.has_value()) {
Diag << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
Tok.getLocation(), NextToken->getLocation()));
diff --git a/clang-tools-extra/clang-tidy/utils/LexerUtils.h b/clang-tools-extra/clang-tidy/utils/LexerUtils.h
index c0993e8872b1e8..afd63885e388ce 100644
--- a/clang-tools-extra/clang-tidy/utils/LexerUtils.h
+++ b/clang-tools-extra/clang-tidy/utils/LexerUtils.h
@@ -89,7 +89,7 @@ SourceLocation findNextAnyTokenKind(SourceLocation Start,
}
}
-std::optional<Token>
+inline std::optional<Token>
findNextTokenIncludingComments(SourceLocation Start, const SourceManager &SM,
const LangOptions &LangOpts) {
return Lexer::findNextToken(Start, SM, LangOpts, true);
>From d2561711a43e3771c56e4a108384697fb3c56fe8 Mon Sep 17 00:00:00 2001
From: Clement Courbet <clement.courbet at gmail.com>
Date: Thu, 16 Jan 2025 08:14:06 +0100
Subject: [PATCH 3/4] Apply suggestions from code review
Apply suggestions
Co-authored-by: cor3ntin <corentinjabot at gmail.com>
---
.../clang-reorder-fields/ReorderFieldsAction.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp b/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
index 40c96f92254e42..30bc8be1719d5a 100644
--- a/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
+++ b/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
@@ -126,11 +126,11 @@ static SourceLocation getEndOfTrailingComment(SourceLocation Loc,
// first comment to be part of the trailing comment.
const unsigned Column = SM.getPresumedColumnNumber(Loc);
std::optional<Token> Tok =
- Lexer::findNextToken(Loc, SM, LangOpts, /*IncludeComments*/ true);
+ Lexer::findNextToken(Loc, SM, LangOpts, /*IncludeComments=*/true);
while (Tok && Tok->is(tok::comment) &&
SM.getPresumedColumnNumber(Tok->getLocation()) > Column) {
Loc = Tok->getEndLoc();
- Tok = Lexer::findNextToken(Loc, SM, LangOpts, /*IncludeComments*/ true);
+ Tok = Lexer::findNextToken(Loc, SM, LangOpts, /*IncludeComments=*/true);
}
return Loc;
}
>From 66acd22f1de097e23eaafe7e5fe20168727f9d82 Mon Sep 17 00:00:00 2001
From: Clement Courbet <courbet at google.com>
Date: Thu, 16 Jan 2025 08:40:24 +0000
Subject: [PATCH 4/4] add unit tests
---
clang/unittests/Lex/LexerTest.cpp | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/clang/unittests/Lex/LexerTest.cpp b/clang/unittests/Lex/LexerTest.cpp
index aead7fb899d0a8..7b058445890a35 100644
--- a/clang/unittests/Lex/LexerTest.cpp
+++ b/clang/unittests/Lex/LexerTest.cpp
@@ -603,6 +603,7 @@ TEST_F(LexerTest, CharRangeOffByOne) {
TEST_F(LexerTest, FindNextToken) {
Lex("int abcd = 0;\n"
+ "// A comment.\n"
"int xyz = abcd;\n");
std::vector<std::string> GeneratedByNextToken;
SourceLocation Loc =
@@ -619,6 +620,25 @@ TEST_F(LexerTest, FindNextToken) {
"xyz", "=", "abcd", ";"));
}
+TEST_F(LexerTest, FindNextTokenIncludingComments) {
+ Lex("int abcd = 0;\n"
+ "// A comment.\n"
+ "int xyz = abcd;\n");
+ std::vector<std::string> GeneratedByNextToken;
+ SourceLocation Loc =
+ SourceMgr.getLocForStartOfFile(SourceMgr.getMainFileID());
+ while (true) {
+ auto T = Lexer::findNextToken(Loc, SourceMgr, LangOpts, true);
+ ASSERT_TRUE(T);
+ if (T->is(tok::eof))
+ break;
+ GeneratedByNextToken.push_back(getSourceText(*T, *T));
+ Loc = T->getLocation();
+ }
+ EXPECT_THAT(GeneratedByNextToken, ElementsAre("abcd", "=", "0", ";", "// A comment.", "int",
+ "xyz", "=", "abcd", ";"));
+}
+
TEST_F(LexerTest, CreatedFIDCountForPredefinedBuffer) {
TrivialModuleLoader ModLoader;
auto PP = CreatePP("", ModLoader);
More information about the cfe-commits
mailing list