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 <cour...@google.com> Date: Wed, 15 Jan 2025 12:19:58 +0000 Subject: [PATCH 1/3] [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 <cour...@google.com> Date: Wed, 15 Jan 2025 15:56:06 +0000 Subject: [PATCH 2/3] 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.cour...@gmail.com> Date: Thu, 16 Jan 2025 08:14:06 +0100 Subject: [PATCH 3/3] Apply suggestions from code review Apply suggestions Co-authored-by: cor3ntin <corentinja...@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; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits