[clangd] Implement simple folding of preprocessor branches (rebased) #121449
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-clang-tools-extra Author: SR_team (sr-tream) ChangesIt's a rebased PR #80592 without pseudo-parser usage. Extract directive branches information from DirectiveTree, fold branches that don't end with eof. Fixes clangd/clangd#1661 Full diff: https://github.com/llvm/llvm-project/pull/121449.diff 3 Files Affected:
diff --git a/clang-tools-extra/clangd/SemanticSelection.cpp b/clang-tools-extra/clangd/SemanticSelection.cpp
index dd7116e619e6d0..57e7ff8b97c658 100644
--- a/clang-tools-extra/clangd/SemanticSelection.cpp
+++ b/clang-tools-extra/clangd/SemanticSelection.cpp
@@ -220,6 +220,24 @@ getFoldingRanges(const std::string &Code, bool LineFoldingOnly) {
auto EndPosition = [&](const Token &T) {
return offsetToPosition(Code, EndOffset(T));
};
+
+ // Preprocessor directives
+ auto PPRanges = pairDirectiveRanges(DirectiveStructure, OrigStream);
+ for (const auto &R : PPRanges) {
+ auto BTok = OrigStream.tokens()[R.Begin];
+ auto ETok = OrigStream.tokens()[R.End];
+ if (ETok.Kind == tok::eof)
+ continue;
+ if (BTok.Line >= ETok.Line)
+ continue;
+
+ Position Start = EndPosition(BTok);
+ Position End = StartPosition(ETok);
+ if (LineFoldingOnly)
+ End.line--;
+ AddFoldingRange(Start, End, FoldingRange::REGION_KIND);
+ }
+
auto Tokens = ParseableStream.tokens();
// Brackets.
for (const auto &Tok : Tokens) {
diff --git a/clang-tools-extra/clangd/support/DirectiveTree.cpp b/clang-tools-extra/clangd/support/DirectiveTree.cpp
index 7ea08add7a107e..2a4448902df539 100644
--- a/clang-tools-extra/clangd/support/DirectiveTree.cpp
+++ b/clang-tools-extra/clangd/support/DirectiveTree.cpp
@@ -356,5 +356,59 @@ TokenStream DirectiveTree::stripDirectives(const TokenStream &In) const {
return Out;
}
+namespace {
+class RangePairer {
+ std::vector<Token::Range> &Ranges;
+
+public:
+ RangePairer(std::vector<Token::Range> &Ranges) : Ranges(Ranges) {}
+
+ void walk(const DirectiveTree &T) {
+ for (const auto &C : T.Chunks)
+ std::visit(*this, C);
+ }
+
+ void operator()(const DirectiveTree::Code &C) {}
+
+ void operator()(const DirectiveTree::Directive &) {}
+
+ void operator()(const DirectiveTree::Conditional &C) {
+ Token::Range Range;
+ Token::Index Last;
+ auto First = true;
+ for (const auto &B : C.Branches) {
+ if (First) {
+ First = false;
+ } else {
+ Range = {Last, B.first.Tokens.Begin};
+ Ranges.push_back(Range);
+ }
+ Last = B.first.Tokens.Begin;
+ }
+ Range = {Last, C.End.Tokens.Begin};
+ Ranges.push_back(Range);
+
+ for (const auto &B : C.Branches)
+ walk(B.second);
+ }
+};
+} // namespace
+
+std::vector<Token::Range> pairDirectiveRanges(const DirectiveTree &Tree,
+ const TokenStream &Code) {
+ std::vector<Token::Range> Ranges;
+ RangePairer(Ranges).walk(Tree);
+
+ // Transform paired ranges to start with last token in its logical line
+ for (auto &R : Ranges) {
+ const Token *Tok = &Code.tokens()[R.Begin + 1];
+ while (Tok->Kind != tok::eof && !Tok->flag(LexFlags::StartsPPLine))
+ ++Tok;
+ Tok = Tok - 1;
+ R.Begin = Tok->OriginalIndex;
+ }
+ return std::move(Ranges);
+}
+
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/support/DirectiveTree.h b/clang-tools-extra/clangd/support/DirectiveTree.h
index 34f5a888863f26..1a377f5e0640ca 100644
--- a/clang-tools-extra/clangd/support/DirectiveTree.h
+++ b/clang-tools-extra/clangd/support/DirectiveTree.h
@@ -124,6 +124,9 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &,
/// The choices are stored in Conditional::Taken nodes.
void chooseConditionalBranches(DirectiveTree &, const TokenStream &Code);
+std::vector<Token::Range> pairDirectiveRanges(const DirectiveTree &Tree,
+ const TokenStream &Code);
+
} // namespace clangd
} // namespace clang
|
|
@llvm/pr-subscribers-clangd Author: SR_team (sr-tream) ChangesIt's a rebased PR #80592 without pseudo-parser usage. Extract directive branches information from DirectiveTree, fold branches that don't end with eof. Fixes clangd/clangd#1661 Full diff: https://github.com/llvm/llvm-project/pull/121449.diff 3 Files Affected:
diff --git a/clang-tools-extra/clangd/SemanticSelection.cpp b/clang-tools-extra/clangd/SemanticSelection.cpp
index dd7116e619e6d0..57e7ff8b97c658 100644
--- a/clang-tools-extra/clangd/SemanticSelection.cpp
+++ b/clang-tools-extra/clangd/SemanticSelection.cpp
@@ -220,6 +220,24 @@ getFoldingRanges(const std::string &Code, bool LineFoldingOnly) {
auto EndPosition = [&](const Token &T) {
return offsetToPosition(Code, EndOffset(T));
};
+
+ // Preprocessor directives
+ auto PPRanges = pairDirectiveRanges(DirectiveStructure, OrigStream);
+ for (const auto &R : PPRanges) {
+ auto BTok = OrigStream.tokens()[R.Begin];
+ auto ETok = OrigStream.tokens()[R.End];
+ if (ETok.Kind == tok::eof)
+ continue;
+ if (BTok.Line >= ETok.Line)
+ continue;
+
+ Position Start = EndPosition(BTok);
+ Position End = StartPosition(ETok);
+ if (LineFoldingOnly)
+ End.line--;
+ AddFoldingRange(Start, End, FoldingRange::REGION_KIND);
+ }
+
auto Tokens = ParseableStream.tokens();
// Brackets.
for (const auto &Tok : Tokens) {
diff --git a/clang-tools-extra/clangd/support/DirectiveTree.cpp b/clang-tools-extra/clangd/support/DirectiveTree.cpp
index 7ea08add7a107e..2a4448902df539 100644
--- a/clang-tools-extra/clangd/support/DirectiveTree.cpp
+++ b/clang-tools-extra/clangd/support/DirectiveTree.cpp
@@ -356,5 +356,59 @@ TokenStream DirectiveTree::stripDirectives(const TokenStream &In) const {
return Out;
}
+namespace {
+class RangePairer {
+ std::vector<Token::Range> &Ranges;
+
+public:
+ RangePairer(std::vector<Token::Range> &Ranges) : Ranges(Ranges) {}
+
+ void walk(const DirectiveTree &T) {
+ for (const auto &C : T.Chunks)
+ std::visit(*this, C);
+ }
+
+ void operator()(const DirectiveTree::Code &C) {}
+
+ void operator()(const DirectiveTree::Directive &) {}
+
+ void operator()(const DirectiveTree::Conditional &C) {
+ Token::Range Range;
+ Token::Index Last;
+ auto First = true;
+ for (const auto &B : C.Branches) {
+ if (First) {
+ First = false;
+ } else {
+ Range = {Last, B.first.Tokens.Begin};
+ Ranges.push_back(Range);
+ }
+ Last = B.first.Tokens.Begin;
+ }
+ Range = {Last, C.End.Tokens.Begin};
+ Ranges.push_back(Range);
+
+ for (const auto &B : C.Branches)
+ walk(B.second);
+ }
+};
+} // namespace
+
+std::vector<Token::Range> pairDirectiveRanges(const DirectiveTree &Tree,
+ const TokenStream &Code) {
+ std::vector<Token::Range> Ranges;
+ RangePairer(Ranges).walk(Tree);
+
+ // Transform paired ranges to start with last token in its logical line
+ for (auto &R : Ranges) {
+ const Token *Tok = &Code.tokens()[R.Begin + 1];
+ while (Tok->Kind != tok::eof && !Tok->flag(LexFlags::StartsPPLine))
+ ++Tok;
+ Tok = Tok - 1;
+ R.Begin = Tok->OriginalIndex;
+ }
+ return std::move(Ranges);
+}
+
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/support/DirectiveTree.h b/clang-tools-extra/clangd/support/DirectiveTree.h
index 34f5a888863f26..1a377f5e0640ca 100644
--- a/clang-tools-extra/clangd/support/DirectiveTree.h
+++ b/clang-tools-extra/clangd/support/DirectiveTree.h
@@ -124,6 +124,9 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &,
/// The choices are stored in Conditional::Taken nodes.
void chooseConditionalBranches(DirectiveTree &, const TokenStream &Code);
+std::vector<Token::Range> pairDirectiveRanges(const DirectiveTree &Tree,
+ const TokenStream &Code);
+
} // namespace clangd
} // namespace clang
|
HighCommander4
requested changes
Jan 17, 2025
There was a problem hiding this comment.
This is my first time looking at this code (SemanticSelection.cpp / DirectiveTree.cpp) so I'm not sure if I'm the best reviewer, but on a high level what this patch is doing seems reasonable to me.
Some unit tests for the change would be great. We have some existing unit tests for folding ranges in SemanticSelectionTests.cpp which you can extend.
Extract directive branches information from DirectiveTree, fold branches that don't end with eof. Fixes clangd/clangd#1661
|
hope you guys are doing well, really need this feature, thank you for your contribution! |
luckzylp
added a commit
to luckzylp/llvm-project
that referenced
this pull request
May 30, 2025
Merge the PR: llvm#121449 by sr-tream and PR: llvm#140959 by aketchum15.
luckzylp
added a commit
to luckzylp/llvm-project
that referenced
this pull request
May 30, 2025
1. Merge the PR: #llvm#121449 by sr-tream and PR: #llvm#140959 by aketchum15. 2. Fix the issue of `ItaniumMangle.cpp` on the linux platform under clang-9 ```c TypeNameOS << Operand.getKind(); ` ` ` The problem of compilation failure caused by the inability to find appropriate operator overloading operations.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It's a rebased PR #80592 without pseudo-parser usage.
Extract directive branches information from DirectiveTree, fold branches that don't end with eof.
Fixes clangd/clangd#1661