Skip to content

[clangd] Implement simple folding for preprocessor branches #140959

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions clang-tools-extra/clangd/SemanticSelection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,24 @@ getFoldingRanges(const std::string &Code, bool LineFoldingOnly) {
auto EndPosition = [&](const Token &T) {
return offsetToPosition(Code, EndOffset(T));
};

// Preprocessor directives
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of minor points here:

  • Since this code block does not use Preprocessed or ParseableStream, while the code blocks for producing the other kinds of regions do, I think it would make sense for code organization to move this block above the declarations of those variables.
  • At the top of this function, please revise the comment // FIXME( usaxena95): Collect PP conditional regions, includes and other code regions ... " to // FIXME( usaxena95): Collect includes and other code regions ...

auto PPRanges = pairDirectiveRanges(DirectiveStructure, OrigStream);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a testcase that has a nested conditional in both branches of an outer conditional, and checks that both nested conditionals get a folding range? (It would be easy to get this wrong by passing ParseableStream rather than OrigStream here, which would have the effect of only processing the branch of the outer conditional which we heuristically decided to treat as active.)

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) {
Expand Down
54 changes: 54 additions & 0 deletions clang-tools-extra/clangd/support/DirectiveTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 Ranges;
}

} // namespace clangd
} // namespace clang
4 changes: 4 additions & 0 deletions clang-tools-extra/clangd/support/DirectiveTree.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &,
/// The choices are stored in Conditional::Taken nodes.
void chooseConditionalBranches(DirectiveTree &, const TokenStream &Code);

/// Pairs preprocessor conditional directives and computes their token ranges.
std::vector<Token::Range> pairDirectiveRanges(const DirectiveTree &Tree,
const TokenStream &Code);

} // namespace clangd
} // namespace clang

Expand Down
20 changes: 20 additions & 0 deletions clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,26 @@ TEST(FoldingRanges, PseudoParserWithoutLineFoldings) {
//[[ foo
/* bar */]]
)cpp",
R"cpp(
//Ignore non-conditional directives
#define A 1

void func() {[[
int Variable = 100;

#ifdef FOO[[
Variable = 1;
]]#else[[
Variable = 2;
//handle nested directives
#if 1[[
Variable = 3;
]]#endif
]]#endif


]]}
)cpp",
};
for (const char *Test : Tests) {
auto T = Annotations(Test);
Expand Down
Loading