nridge updated this revision to Diff 526409.
nridge added a comment.
slight simplification
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D151190/new/
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
@@ -1332,26 +1332,31 @@
#define PREAMBLEMACRO 42
#if PREAMBLEMACRO > 40
#define ACTIVE
-$inactive1[[#else
- #define INACTIVE
-#endif]]
+#else
+$inactive1[[ #define INACTIVE]]
+#endif
int endPreamble;
-$inactive2[[#ifndef CMDMACRO
- int inactiveInt;
-#endif]]
+#ifndef CMDMACRO
+$inactive2[[ int inactiveInt;]]
+#endif
#undef CMDMACRO
-$inactive3[[#ifdef CMDMACRO
- int inactiveInt2;
-#else]]
- int activeInt;
+#ifdef CMDMACRO
+$inactive3[[ int inactiveInt2;]]
+#elif PREAMBLEMACRO > 0
+ int activeInt1;
+ int activeInt2;
+#else
+$inactive4[[ int inactiveInt3;]]
#endif
+#ifdef CMDMACRO
+#endif // empty inactive range, gets dropped
)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,41 @@
ServerCallbacks->onDiagnosticsReady(Path, AST.version(),
std::move(Diagnostics));
if (CollectInactiveRegions) {
- ServerCallbacks->onInactiveRegionsReady(
- Path, std::move(AST.getMacros().SkippedRanges));
+ std::vector<Range> SkippedRanges(
+ std::move(AST.getMacros().SkippedRanges));
+ const auto &SM = AST.getSourceManager();
+ StringRef MainCode =
+ SM.getBufferOrFake(SM.getMainFileID()).getBuffer();
+ std::vector<Range> InactiveRegions;
+ for (const Range &Skipped : SkippedRanges) {
+ Range Inactive = Skipped;
+ // 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.
+ if (Inactive.end.character == 0 && Inactive.end.line > 0) {
+ --Inactive.end.line;
+ }
+ // Exclude the directive lines themselves from the range.
+ if (Inactive.end.line >= Inactive.start.line + 2) {
+ ++Inactive.start.line;
+ --Inactive.end.line;
+ } else {
+ // range would be empty, e.g. #endif on next line after #ifdef
+ continue;
+ }
+ // Since we've adjusted the ending line, we need to recompute the
+ // column to reflect the end of that line.
+ if (auto EndOfLine = endOfLine(MainCode, Inactive.end.line)) {
+ Inactive.end = *EndOfLine;
+ } else {
+ elog("Failed to determine end of line: {0}",
+ EndOfLine.takeError());
+ continue;
+ }
+ InactiveRegions.push_back(Inactive);
+ }
+ ServerCallbacks->onInactiveRegionsReady(Path, InactiveRegions);
}
});
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
