diff --git a/clang-tools-extra/clangd/CollectMacros.h b/clang-tools-extra/clangd/CollectMacros.h --- a/clang-tools-extra/clangd/CollectMacros.h +++ b/clang-tools-extra/clangd/CollectMacros.h @@ -99,6 +99,21 @@ MainFileMacros &Out; }; +/// Represents a `#pragma mark` in the main file. +/// +/// There can be at most one pragma mark per line. +struct PragmaMark { + SourceLocation Loc; + std::string Text; + bool IsGroup; + + DocumentSymbol toSymbol(const SourceManager &SM) const; +}; + +/// Collect all pragma marks from the main file. +std::unique_ptr +collectPragmaMarksCallback(const SourceManager &, std::vector &Out); + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/CollectMacros.cpp b/clang-tools-extra/clangd/CollectMacros.cpp --- a/clang-tools-extra/clangd/CollectMacros.cpp +++ b/clang-tools-extra/clangd/CollectMacros.cpp @@ -10,6 +10,39 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Lex/Lexer.h" +namespace { +class CollectPragmaMarks : public clang::PPCallbacks { +public: + explicit CollectPragmaMarks(const clang::SourceManager &SM, + std::vector &Out) + : SM(SM), Out(Out) {} + void PragmaMark(clang::SourceLocation Loc, clang::StringRef Trivia) override { + if (clang::clangd::isInsideMainFile(Loc, SM)) { + clang::StringRef Name = Trivia.trim(); + bool IsGroup = false; + // "-\s+" or "" after an initial trim. The former is + // considered a group, the latter just a mark. We need to include a name + // here since editors won't properly render the symbol otherwise. + clang::StringRef MaybeGroupName = Name; + if (MaybeGroupName.consume_front("-") && + (MaybeGroupName.ltrim() != MaybeGroupName || + MaybeGroupName.empty())) { + Name = + MaybeGroupName.empty() ? "(unnamed group)" : MaybeGroupName.ltrim(); + IsGroup = true; + } else if (Name.empty()) { + Name = "(unnamed mark)"; + } + Out.emplace_back(clang::clangd::PragmaMark{Loc, Name.str(), IsGroup}); + } + } + +private: + const clang::SourceManager &SM; + std::vector &Out; +}; +} // namespace + namespace clang { namespace clangd { @@ -30,5 +63,23 @@ else Out.UnknownMacros.push_back({Range, IsDefinition}); } + +DocumentSymbol PragmaMark::toSymbol(const SourceManager &SM) const { + DocumentSymbol Sym; + Sym.name = Text; + Sym.kind = SymbolKind::File; + Position Start = sourceLocToPosition(SM, Loc); + Position End = {Start.line + 1, 0}; + Sym.range = Range{Start, End}; + Sym.selectionRange = Range{Start, End}; + return Sym; +} + +std::unique_ptr +collectPragmaMarksCallback(const SourceManager &SM, + std::vector &Out) { + return std::make_unique(SM, Out); +} + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/FindSymbols.cpp b/clang-tools-extra/clangd/FindSymbols.cpp --- a/clang-tools-extra/clangd/FindSymbols.cpp +++ b/clang-tools-extra/clangd/FindSymbols.cpp @@ -289,6 +289,10 @@ /// - visiting decls is actually simple, so we don't hit the complicated /// cases that RAV mostly helps with (types, expressions, etc.) class DocumentOutline { + struct TextMarkSymbol { + DocumentSymbol DocSym; + bool IsGroup; + }; // A DocumentSymbol we're constructing. // We use this instead of DocumentSymbol directly so that we can keep track // of the nodes we insert for macros. @@ -303,16 +307,99 @@ DocumentSymbol build() && { for (SymBuilder &C : Children) { Symbol.children.push_back(std::move(C).build()); + } + return std::move(Symbol); + } + + /// Fix up ranges so each parent's range contains all of its descendants. + void standardizeRanges() { + for (SymBuilder &C : Children) { + C.standardizeRanges(); // Expand range to ensure children nest properly, which editors expect. // This can fix some edge-cases in the AST, but is vital for macros. // A macro expansion "contains" AST node if it covers the node's primary // location, but it may not span the node's whole range. - Symbol.range.start = - std::min(Symbol.range.start, Symbol.children.back().range.start); - Symbol.range.end = - std::max(Symbol.range.end, Symbol.children.back().range.end); + Symbol.range.start = std::min(Symbol.range.start, C.Symbol.range.start); + Symbol.range.end = std::max(Symbol.range.end, C.Symbol.range.end); + } + // FIXME: We should also make sure nodes are properly sorted and nested + // here. + } + + /// Merge in `TextMarks` which are sorted in descending order by line. + /// + /// This assumes this `SymBuilder` has standardized ranges and is + /// recursively sorted in ascending order by range. + void mergeTextMarks(std::vector &TextMarks, bool IsRoot) { + size_t LastGroupIndex = 0; + size_t ChildIndex = 0; + bool HasOpenGroup = false; + while (!TextMarks.empty()) { + const auto &TextMark = TextMarks.back(); + // If we don't contain the mark, we can break early, since it must + // be in a parent node instead. Note that we can still have children + // that need to be added to the last TextMark group though. + if (!Symbol.range.contains(TextMark.DocSym.range) && !IsRoot) + break; + + while (ChildIndex < Children.size()) { + auto &C = Children[ChildIndex]; + + // Next mark is contained in the child, we need to recurse to let the + // child handle it. + if (C.Symbol.range.contains(TextMark.DocSym.range)) { + C.mergeTextMarks(TextMarks, /** IsRoot= */ false); + break; + } + // Text mark is after the child node, so we can process the child. + if (C.Symbol.range < TextMark.DocSym.range) { + if (HasOpenGroup) { // Child belongs to the previous TextMark group. + Children[LastGroupIndex].Children.push_back(C); + unionRanges(Children[LastGroupIndex].Symbol.range, + C.Symbol.range); + Children.erase(Children.begin() + ChildIndex); + } else { // Child doesn't belong to a TextMark group. Skip it. + ChildIndex++; + } + continue; + } + // Text mark is before the child node. + SymBuilder Node; + Node.EnclosingMacroLoc = C.EnclosingMacroLoc; + Node.Symbol = TextMark.DocSym; + Children.insert(Children.begin() + ChildIndex, Node); + if (TextMark.IsGroup) { + LastGroupIndex = ChildIndex; + HasOpenGroup = true; + } else { // New text mark ends the previous group if one existed. + // FIXME: Consider letting non-groups nest inside a group. + HasOpenGroup = false; + } + TextMarks.pop_back(); + ChildIndex++; + break; + } + // Reached the end of our children but still have TextMark(s) left, + // they should just be normal children. + if (ChildIndex == Children.size()) { + SymBuilder Node; + Node.EnclosingMacroLoc = EnclosingMacroLoc; + Node.Symbol = TextMark.DocSym; + Children.push_back(Node); + ChildIndex++; + TextMarks.pop_back(); + } + } + // If we have a group open but still have children remaining, they belong + // to that group instead. + if (HasOpenGroup && ChildIndex < Children.size()) { + while (ChildIndex < Children.size()) { + auto &C = Children[ChildIndex]; + Children[LastGroupIndex].Children.push_back(C); + unionRanges(Children[LastGroupIndex].Symbol.range, C.Symbol.range); + Children.erase(Children.begin() + ChildIndex); + } } - return std::move(Symbol); } // Add a symbol as a child of the current one. @@ -379,6 +466,19 @@ SymBuilder Root; for (auto &TopLevel : AST.getLocalTopLevelDecls()) traverseDecl(TopLevel, Root); + + // We need to fix up the ranges before adding TextMarks to be sure we add + // them in the correct spots. + Root.standardizeRanges(); + + const auto &SM = AST.getSourceManager(); + const auto &TextMarks = AST.getMarks(); + std::vector TextMarkSyms; + for (auto It = TextMarks.rbegin(); It != TextMarks.rend(); ++It) { + TextMarkSyms.push_back({It->toSymbol(SM), It->IsGroup}); + } + Root.mergeTextMarks(TextMarkSyms, /** IsRoot= */ true); + return std::move(std::move(Root).build().children); } diff --git a/clang-tools-extra/clangd/FindTarget.cpp b/clang-tools-extra/clangd/FindTarget.cpp --- a/clang-tools-extra/clangd/FindTarget.cpp +++ b/clang-tools-extra/clangd/FindTarget.cpp @@ -652,7 +652,7 @@ void visitProtocolList( llvm::iterator_range Protocols, llvm::iterator_range Locations) { - for (const auto &P : llvm::zip(Protocols, Locations)) { + for (auto P : llvm::zip(Protocols, Locations)) { Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(), std::get<1>(P), /*IsDecl=*/false, diff --git a/clang-tools-extra/clangd/ParsedAST.h b/clang-tools-extra/clangd/ParsedAST.h --- a/clang-tools-extra/clangd/ParsedAST.h +++ b/clang-tools-extra/clangd/ParsedAST.h @@ -101,6 +101,8 @@ /// Gets all macro references (definition, expansions) present in the main /// file, including those in the preamble region. const MainFileMacros &getMacros() const; + /// Gets all pragma marks in the main file. + const std::vector &getMarks() const; /// Tokens recorded while parsing the main file. /// (!) does not have tokens from the preamble. const syntax::TokenBuffer &getTokens() const { return Tokens; } @@ -121,7 +123,8 @@ std::shared_ptr Preamble, std::unique_ptr Clang, std::unique_ptr Action, syntax::TokenBuffer Tokens, - MainFileMacros Macros, std::vector LocalTopLevelDecls, + MainFileMacros Macros, std::vector Marks, + std::vector LocalTopLevelDecls, llvm::Optional> Diags, IncludeStructure Includes, CanonicalIncludes CanonIncludes); @@ -144,6 +147,8 @@ /// All macro definitions and expansions in the main file. MainFileMacros Macros; + // Pragma marks in the main file. + std::vector Marks; // Data, stored after parsing. None if AST was built with a stale preamble. llvm::Optional> Diags; // Top-level decls inside the current file. Not that this does not include diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -436,6 +436,12 @@ std::make_unique(Clang->getSourceManager(), Macros)); + std::vector Marks; + if (Preamble) + Marks = Preamble->Marks; + Clang->getPreprocessor().addPPCallbacks( + collectPragmaMarksCallback(Clang->getSourceManager(), Marks)); + // Copy over the includes from the preamble, then combine with the // non-preamble includes below. CanonicalIncludes CanonIncludes; @@ -497,7 +503,7 @@ } return ParsedAST(Inputs.Version, std::move(Preamble), std::move(Clang), std::move(Action), std::move(Tokens), std::move(Macros), - std::move(ParsedDecls), std::move(Diags), + std::move(Marks), std::move(ParsedDecls), std::move(Diags), std::move(Includes), std::move(CanonIncludes)); } @@ -537,6 +543,7 @@ } const MainFileMacros &ParsedAST::getMacros() const { return Macros; } +const std::vector &ParsedAST::getMarks() const { return Marks; } std::size_t ParsedAST::getUsedBytes() const { auto &AST = getASTContext(); @@ -583,12 +590,14 @@ std::unique_ptr Clang, std::unique_ptr Action, syntax::TokenBuffer Tokens, MainFileMacros Macros, + std::vector Marks, std::vector LocalTopLevelDecls, llvm::Optional> Diags, IncludeStructure Includes, CanonicalIncludes CanonIncludes) : Version(Version), Preamble(std::move(Preamble)), Clang(std::move(Clang)), Action(std::move(Action)), Tokens(std::move(Tokens)), - Macros(std::move(Macros)), Diags(std::move(Diags)), + Macros(std::move(Macros)), Marks(std::move(Marks)), + Diags(std::move(Diags)), LocalTopLevelDecls(std::move(LocalTopLevelDecls)), Includes(std::move(Includes)), CanonIncludes(std::move(CanonIncludes)) { Resolver = std::make_unique(getASTContext()); diff --git a/clang-tools-extra/clangd/Preamble.h b/clang-tools-extra/clangd/Preamble.h --- a/clang-tools-extra/clangd/Preamble.h +++ b/clang-tools-extra/clangd/Preamble.h @@ -61,6 +61,8 @@ // Users care about headers vs main-file, not preamble vs non-preamble. // These should be treated as main-file entities e.g. for code completion. MainFileMacros Macros; + // Pragma marks defined in the preamble section of the main file. + std::vector Marks; // Cache of FS operations performed when building the preamble. // When reusing a preamble, this cache can be consumed to save IO. std::unique_ptr StatCache; diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/clang-tools-extra/clangd/Preamble.cpp @@ -73,6 +73,8 @@ MainFileMacros takeMacros() { return std::move(Macros); } + std::vector takeMarks() { return std::move(Marks); } + CanonicalIncludes takeCanonicalIncludes() { return std::move(CanonIncludes); } bool isMainFileIncludeGuarded() const { return IsMainFileIncludeGuarded; } @@ -103,7 +105,9 @@ return std::make_unique( collectIncludeStructureCallback(*SourceMgr, &Includes), - std::make_unique(*SourceMgr, Macros)); + std::make_unique( + std::make_unique(*SourceMgr, Macros), + collectPragmaMarksCallback(*SourceMgr, Marks))); } CommentHandler *getCommentHandler() override { @@ -130,6 +134,7 @@ IncludeStructure Includes; CanonicalIncludes CanonIncludes; MainFileMacros Macros; + std::vector Marks; bool IsMainFileIncludeGuarded = false; std::unique_ptr IWYUHandler = nullptr; const clang::LangOptions *LangOpts = nullptr; @@ -387,6 +392,7 @@ Result->Diags = std::move(Diags); Result->Includes = CapturedInfo.takeIncludes(); Result->Macros = CapturedInfo.takeMacros(); + Result->Marks = CapturedInfo.takeMarks(); Result->CanonIncludes = CapturedInfo.takeCanonicalIncludes(); Result->StatCache = std::move(StatCache); Result->MainIsIncludeGuarded = CapturedInfo.isMainFileIncludeGuarded(); diff --git a/clang-tools-extra/clangd/SourceCode.h b/clang-tools-extra/clangd/SourceCode.h --- a/clang-tools-extra/clangd/SourceCode.h +++ b/clang-tools-extra/clangd/SourceCode.h @@ -129,6 +129,9 @@ // Note that clang also uses closed source ranges, which this can't handle! Range halfOpenToRange(const SourceManager &SM, CharSourceRange R); +// Expand range `A` to also contain `B`. +void unionRanges(Range &A, Range B); + // Converts an offset to a clang line/column (1-based, columns are bytes). // The offset must be in range [0, Code.size()]. // Prefer to use SourceManager if one is available. diff --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp --- a/clang-tools-extra/clangd/SourceCode.cpp +++ b/clang-tools-extra/clangd/SourceCode.cpp @@ -471,6 +471,13 @@ return {Begin, End}; } +void unionRanges(Range &A, Range B) { + if (B.start < A.start) + A.start = B.start; + if (A.end < B.end) + A.end = B.end; +} + std::pair offsetToClangLineColumn(llvm::StringRef Code, size_t Offset) { Offset = std::min(Code.size(), Offset); diff --git a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp --- a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp @@ -1027,6 +1027,105 @@ AllOf(WithName("-pur"), WithKind(SymbolKind::Method)))))); } +TEST(DocumentSymbolsTest, PragmaMarkGroups) { + TestTU TU; + TU.ExtraArgs = {"-xobjective-c++", "-Wno-objc-root-class"}; + Annotations Main(R"cpp( + $DogDef[[@interface Dog + @end]] + + $DogImpl[[@implementation Dog + + + (id)sharedDoggo { return 0; } + + #pragma $Overrides[[mark - Overrides + + - (id)init { + return self; + } + - (void)bark {}]] + + #pragma $Specifics[[mark - Dog Specifics + + - (int)isAGoodBoy { + return 1; + }]] + @]]end // FIXME: Why doesn't this include the 'end'? + + #pragma $End[[mark - End +]] + )cpp"); + TU.Code = Main.code().str(); + EXPECT_THAT( + getSymbols(TU.build()), + ElementsAre( + AllOf(WithName("Dog"), SymRange(Main.range("DogDef"))), + AllOf(WithName("Dog"), SymRange(Main.range("DogImpl")), + Children(AllOf(WithName("+sharedDoggo"), + WithKind(SymbolKind::Method)), + AllOf(WithName("Overrides"), + SymRange(Main.range("Overrides")), + Children(AllOf(WithName("-init"), + WithKind(SymbolKind::Method)), + AllOf(WithName("-bark"), + WithKind(SymbolKind::Method)))), + AllOf(WithName("Dog Specifics"), + SymRange(Main.range("Specifics")), + Children(AllOf(WithName("-isAGoodBoy"), + WithKind(SymbolKind::Method)))))), + AllOf(WithName("End"), SymRange(Main.range("End"))))); +} + +TEST(DocumentSymbolsTest, PragmaMarkGroupsNoNesting) { + TestTU TU; + TU.ExtraArgs = {"-xobjective-c++", "-Wno-objc-root-class"}; + Annotations Main(R"cpp( + #pragma mark Helpers + void helpA(id obj) {} + + #pragma mark - + #pragma mark Core + + void coreMethod() {} + )cpp"); + TU.Code = Main.code().str(); + EXPECT_THAT(getSymbols(TU.build()), + ElementsAre(AllOf(WithName("Helpers")), AllOf(WithName("helpA")), + AllOf(WithName("(unnamed group)")), + AllOf(WithName("Core")), + AllOf(WithName("coreMethod")))); +} + +TEST(DocumentSymbolsTest, SymbolsAreSorted) { + TestTU TU; + TU.ExtraArgs = {"-xobjective-c++", "-Wno-objc-root-class"}; + Annotations Main(R"cpp( + @interface MYObject + @end + + void someFunctionAbove() {} + + @implementation MYObject + - (id)init { return self; } + + void someHelperFunction() {} + + - (void)retain {} + - (void)release {} + @end + )cpp"); + TU.Code = Main.code().str(); + EXPECT_THAT(getSymbols(TU.build()), + ElementsAre(AllOf(WithName("MYObject")), + AllOf(WithName("someFunctionAbove")), + // FIXME: This should be nested under MYObject below. + AllOf(WithName("someHelperFunction")), + AllOf(WithName("MYObject"), + Children(AllOf(WithName("-init")), + AllOf(WithName("-retain")), + AllOf(WithName("-release")))))); +} + } // namespace } // namespace clangd } // namespace clang diff --git a/clang/include/clang/Lex/PPCallbacks.h b/clang/include/clang/Lex/PPCallbacks.h --- a/clang/include/clang/Lex/PPCallbacks.h +++ b/clang/include/clang/Lex/PPCallbacks.h @@ -492,6 +492,11 @@ Second->PragmaComment(Loc, Kind, Str); } + void PragmaMark(SourceLocation Loc, StringRef Trivia) override { + First->PragmaMark(Loc, Trivia); + Second->PragmaMark(Loc, Trivia); + } + void PragmaDetectMismatch(SourceLocation Loc, StringRef Name, StringRef Value) override { First->PragmaDetectMismatch(Loc, Name, Value);