nridge created this revision.
nridge added a reviewer: hokein.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
nridge requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
This carries over the fix previously made for semantic highlighting
https://reviews.llvm.org/D92148, to the new inactiveRegions
protocol as well.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D151190
Files:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/SourceCode.cpp
clang-tools-extra/clangd/SourceCode.h
clang-tools-extra/clangd/unittests/ClangdTests.cpp
Index: clang-tools-extra/clangd/unittests/ClangdTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ClangdTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdTests.cpp
@@ -1342,16 +1342,19 @@
#undef CMDMACRO
$inactive3[[#ifdef CMDMACRO
int inactiveInt2;
-#else]]
- int activeInt;
-#endif
+#elif PREAMBLEMACRO > 0]]
+ int activeInt1;
+ int activeInt2;
+$inactive4[[#else
+ int inactiveInt3;
+#endif]]
)cpp");
Server.addDocument(testPath("foo.cpp"), Source.code());
ASSERT_TRUE(Server.blockUntilIdleForTest());
EXPECT_THAT(Callback.FoundInactiveRegions,
- ElementsAre(ElementsAre(Source.range("inactive1"),
- Source.range("inactive2"),
- Source.range("inactive3"))));
+ ElementsAre(ElementsAre(
+ Source.range("inactive1"), Source.range("inactive2"),
+ Source.range("inactive3"), Source.range("inactive4"))));
}
} // namespace
Index: clang-tools-extra/clangd/SourceCode.h
===================================================================
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -72,6 +72,9 @@
/// FIXME: This should return an error if the location is invalid.
Position sourceLocToPosition(const SourceManager &SM, SourceLocation Loc);
+/// Get the last Position on a given line.
+llvm::Expected<Position> endOfLine(llvm::StringRef Code, int Line);
+
/// Return the file location, corresponding to \p P. Note that one should take
/// care to avoid comparing the result with expansion locations.
llvm::Expected<SourceLocation> sourceLocationInMainFile(const SourceManager &SM,
Index: clang-tools-extra/clangd/SourceCode.cpp
===================================================================
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -228,6 +228,16 @@
return P;
}
+llvm::Expected<Position> endOfLine(llvm::StringRef Code, int Line) {
+ auto StartOfLine = positionToOffset(Code, Position{Line, 0});
+ if (!StartOfLine)
+ return StartOfLine.takeError();
+ StringRef LineText = Code.drop_front(*StartOfLine).take_until([](char C) {
+ return C == '\n';
+ });
+ return Position{Line, static_cast<int>(lspLength(LineText))};
+}
+
bool isSpelledInSource(SourceLocation Loc, const SourceManager &SM) {
if (Loc.isFileID())
return true;
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===================================================================
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -483,22 +483,15 @@
for (; It != NonConflicting.end() && It->R.start.line < Line; ++It)
WithInactiveLines.push_back(std::move(*It));
// Add a token for the inactive line itself.
- auto StartOfLine = positionToOffset(MainCode, Position{Line, 0});
- if (StartOfLine) {
- StringRef LineText =
- MainCode.drop_front(*StartOfLine).take_until([](char C) {
- return C == '\n';
- });
+ auto EndOfLine = endOfLine(MainCode, Line);
+ if (EndOfLine) {
HighlightingToken HT;
WithInactiveLines.emplace_back();
WithInactiveLines.back().Kind = HighlightingKind::InactiveCode;
WithInactiveLines.back().R.start.line = Line;
- WithInactiveLines.back().R.end.line = Line;
- WithInactiveLines.back().R.end.character =
- static_cast<int>(lspLength(LineText));
+ WithInactiveLines.back().R.end = *EndOfLine;
} else {
- elog("Failed to convert position to offset: {0}",
- StartOfLine.takeError());
+ elog("Failed to determine end of line: {0}", EndOfLine.takeError());
}
// Skip any other tokens on the inactive line. e.g.
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -116,8 +116,26 @@
ServerCallbacks->onDiagnosticsReady(Path, AST.version(),
std::move(Diagnostics));
if (CollectInactiveRegions) {
- ServerCallbacks->onInactiveRegionsReady(
- Path, std::move(AST.getMacros().SkippedRanges));
+ std::vector<Range> InactiveRegions(
+ std::move(AST.getMacros().SkippedRanges));
+ // Sometimes, SkippedRanges contains a range ending at position 0
+ // of a line. Clients that apply whole-line styles will treat that
+ // line as inactive which is not desirable, so adjust the ending
+ // position to be the end of the previous line.
+ const auto &SM = AST.getSourceManager();
+ StringRef MainCode =
+ SM.getBufferOrFake(SM.getMainFileID()).getBuffer();
+ for (Range &R : InactiveRegions) {
+ if (R.end.character == 0 && R.end.line > 0) {
+ if (auto EndOfLine = endOfLine(MainCode, R.end.line - 1)) {
+ R.end = *EndOfLine;
+ } else {
+ elog("Failed to determine end of line: {0}",
+ EndOfLine.takeError());
+ }
+ }
+ }
+ ServerCallbacks->onInactiveRegionsReady(Path, InactiveRegions);
}
});
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
