diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -116,8 +116,8 @@ ServerCallbacks->onDiagnosticsReady(Path, AST.version(), std::move(Diagnostics)); if (CollectInactiveRegions) { - ServerCallbacks->onInactiveRegionsReady( - Path, std::move(AST.getMacros().SkippedRanges)); + ServerCallbacks->onInactiveRegionsReady(Path, + getInactiveRegions(AST)); } }); } diff --git a/clang-tools-extra/clangd/SemanticHighlighting.h b/clang-tools-extra/clangd/SemanticHighlighting.h --- a/clang-tools-extra/clangd/SemanticHighlighting.h +++ b/clang-tools-extra/clangd/SemanticHighlighting.h @@ -120,6 +120,11 @@ std::vector diffTokens(llvm::ArrayRef Before, llvm::ArrayRef After); +// Returns ranges of the file that are inside an inactive preprocessor branch. +// The preprocessor directives at the beginning and end of a branch themselves +// are not included. +std::vector getInactiveRegions(ParsedAST &AST); + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -39,6 +39,17 @@ namespace clangd { namespace { +/// Get the last Position on a given line. +llvm::Expected 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(lspLength(LineText))}; +} + /// Some names are not written in the source code and cannot be highlighted, /// e.g. anonymous classes. This function detects those cases. bool canHighlightName(DeclarationName Name) { @@ -516,38 +527,27 @@ // Merge token stream with "inactive line" markers. std::vector WithInactiveLines; - auto SortedSkippedRanges = AST.getMacros().SkippedRanges; - llvm::sort(SortedSkippedRanges); + auto SortedInactiveRegions = getInactiveRegions(AST); + llvm::sort(SortedInactiveRegions); auto It = NonConflicting.begin(); - for (const Range &R : SortedSkippedRanges) { - // Create one token for each line in the skipped range, so it works + for (const Range &R : SortedInactiveRegions) { + // Create one token for each line in the inactive 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) { - // If the end of the inactive range is at the beginning - // of a line, that line is not inactive. - if (Line == R.end.line && R.end.character == 0) - continue; // Copy tokens before the inactive line 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(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. @@ -1544,5 +1544,40 @@ return {std::move(Edit)}; } +std::vector getInactiveRegions(ParsedAST &AST) { + std::vector SkippedRanges(std::move(AST.getMacros().SkippedRanges)); + const auto &SM = AST.getSourceManager(); + StringRef MainCode = SM.getBufferOrFake(SM.getMainFileID()).getBuffer(); + std::vector 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); + } + return InactiveRegions; +} + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/unittests/ClangdTests.cpp b/clang-tools-extra/clangd/unittests/ClangdTests.cpp --- a/clang-tools-extra/clangd/unittests/ClangdTests.cpp +++ b/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 diff --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp --- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp +++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp @@ -451,11 +451,11 @@ #define $Macro_decl[[test]] #undef $Macro[[test]] -$InactiveCode[[#ifdef test]] -$InactiveCode[[#endif]] + #ifdef $Macro[[test]] + #endif -$InactiveCode[[#if defined(test)]] -$InactiveCode[[#endif]] + #if defined($Macro[[test]]) + #endif )cpp", R"cpp( struct $Class_def[[S]] { @@ -562,8 +562,9 @@ R"cpp( // Code in the preamble. // Inactive lines get an empty InactiveCode token at the beginning. -$InactiveCode[[#ifdef test]] -$InactiveCode[[#endif]] + #ifdef $Macro[[test]] +$InactiveCode[[int Inactive1;]] + #endif // A declaration to cause the preamble to end. int $Variable_def[[EndPreamble]]; @@ -572,21 +573,21 @@ // Code inside inactive blocks does not get regular highlightings // because it's not part of the AST. #define $Macro_decl[[test2]] -$InactiveCode[[#if defined(test)]] + #if defined($Macro[[test]]) $InactiveCode[[int Inactive2;]] -$InactiveCode[[#elif defined(test2)]] + #elif defined($Macro[[test2]]) int $Variable_def[[Active1]]; -$InactiveCode[[#else]] + #else $InactiveCode[[int Inactive3;]] -$InactiveCode[[#endif]] + #endif #ifndef $Macro[[test]] int $Variable_def[[Active2]]; #endif -$InactiveCode[[#ifdef test]] + #ifdef $Macro[[test]] $InactiveCode[[int Inactive4;]] -$InactiveCode[[#else]] + #else int $Variable_def[[Active3]]; #endif )cpp",