hokein updated this revision to Diff 284715.
hokein marked 3 inline comments as done.
hokein added a comment.
address review comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85635/new/
https://reviews.llvm.org/D85635
Files:
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -503,11 +503,11 @@
#define $Macro[[test]]
#undef $Macro[[test]]
-$InactiveCode[[]] #ifdef $Macro[[test]]
-$InactiveCode[[]] #endif
+$InactiveCode[[#ifdef test]]
+$InactiveCode[[#endif]]
-$InactiveCode[[]] #if defined($Macro[[test]])
-$InactiveCode[[]] #endif
+$InactiveCode[[#if defined(test)]]
+$InactiveCode[[#endif]]
)cpp",
R"cpp(
struct $Class[[S]] {
@@ -614,8 +614,8 @@
R"cpp(
// Code in the preamble.
// Inactive lines get an empty InactiveCode token at the beginning.
-$InactiveCode[[]] #ifdef $Macro[[test]]
-$InactiveCode[[]] #endif
+$InactiveCode[[#ifdef test]]
+$InactiveCode[[#endif]]
// A declaration to cause the preamble to end.
int $Variable[[EndPreamble]];
@@ -623,17 +623,17 @@
// Code after the preamble.
// Code inside inactive blocks does not get regular highlightings
// because it's not part of the AST.
-$InactiveCode[[]] #ifdef $Macro[[test]]
-$InactiveCode[[]] int Inactive2;
-$InactiveCode[[]] #endif
+$InactiveCode[[#ifdef test]]
+$InactiveCode[[int Inactive2;]]
+$InactiveCode[[#endif]]
#ifndef $Macro[[test]]
int $Variable[[Active1]];
#endif
-$InactiveCode[[]] #ifdef $Macro[[test]]
-$InactiveCode[[]] int Inactive3;
-$InactiveCode[[]] #else
+$InactiveCode[[#ifdef test]]
+$InactiveCode[[int Inactive3;]]
+$InactiveCode[[#else]]
int $Variable[[Active2]];
#endif
)cpp",
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===================================================================
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -221,23 +221,51 @@
// the end of the Tokens).
TokRef = TokRef.drop_front(Conflicting.size());
}
+ const auto &SM = AST.getSourceManager();
+ StringRef MainCode = SM.getBuffer(SM.getMainFileID())->getBuffer();
// Add tokens indicating lines skipped by the preprocessor.
for (const Range &R : AST.getMacros().SkippedRanges) {
// Create one token for each line in the skipped range, so it works
// with line-based diffing.
assert(R.start.line <= R.end.line);
for (int Line = R.start.line; Line <= R.end.line; ++Line) {
- // Don't bother computing the offset for the end of the line, just use
- // zero. The client will treat this highlighting kind specially, and
- // highlight the entire line visually (i.e. not just to where the text
- // on the line ends, but to the end of the screen).
- NonConflicting.push_back({HighlightingKind::InactiveCode,
- {Position{Line, 0}, Position{Line, 0}}});
+ auto StartOfLine = positionToOffset(MainCode, Position{Line, 0});
+ if (!StartOfLine) {
+ elog("Failed to convert position to offset: {0}",
+ StartOfLine.takeError());
+ continue;
+ }
+ StringRef LineText =
+ MainCode.drop_front(*StartOfLine).take_until([](char C) {
+ return C == '\n';
+ });
+ NonConflicting.push_back(
+ {HighlightingKind::InactiveCode,
+ {Position{Line, 0},
+ Position{Line, static_cast<int>(lspLength(LineText))}}});
}
}
// Re-sort the tokens because that's what the diffing expects.
llvm::sort(NonConflicting);
- return NonConflicting;
+ std::vector<HighlightingToken> Results;
+ // Remove tokens that are overlapped with an inactive region, e.g.
+ // `#ifndef Foo` is considered as part of an inactive region when Foo is
+ // defined, and there is a Foo macro token.
+ // FIXME: we should reduce the scope of the inactive region to not include
+ // the directive itself.
+ for (ArrayRef<HighlightingToken> TokRef = NonConflicting;
+ !TokRef.empty();) {
+ ArrayRef<HighlightingToken> ContainedTokens =
+ TokRef.take_while([&](const HighlightingToken &T) {
+ return TokRef.front().R.contains(T.R);
+ });
+ assert(ContainedTokens.size() == 1 ||
+ (ContainedTokens.size() > 1 &&
+ TokRef.front().Kind == HighlightingKind::InactiveCode));
+ Results.push_back(ContainedTokens.front());
+ TokRef = TokRef.drop_front(ContainedTokens.size());
+ }
+ return Results;
}
private:
@@ -493,9 +521,6 @@
std::vector<SemanticToken> Result;
const HighlightingToken *Last = nullptr;
for (const HighlightingToken &Tok : Tokens) {
- // FIXME: support inactive code - we need to provide the actual bounds.
- if (Tok.Kind == HighlightingKind::InactiveCode)
- continue;
Result.emplace_back();
SemanticToken &Out = Result.back();
// deltaStart/deltaLine are relative if possible.
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
