This revision was automatically updated to reflect the committed changes. Closed by commit rGd75fb1ee794e: [clangd] Support `#pragma mark` in the outline (authored by dgoldman).
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105904/new/ https://reviews.llvm.org/D105904 Files: clang-tools-extra/clangd/CollectMacros.cpp clang-tools-extra/clangd/CollectMacros.h clang-tools-extra/clangd/FindSymbols.cpp clang-tools-extra/clangd/ParsedAST.cpp clang-tools-extra/clangd/ParsedAST.h clang-tools-extra/clangd/Preamble.cpp clang-tools-extra/clangd/Preamble.h clang-tools-extra/clangd/SourceCode.cpp clang-tools-extra/clangd/SourceCode.h clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp clang-tools-extra/clangd/unittests/ParsedASTTests.cpp clang/include/clang/Lex/PPCallbacks.h
Index: clang/include/clang/Lex/PPCallbacks.h =================================================================== --- clang/include/clang/Lex/PPCallbacks.h +++ 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); Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp +++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp @@ -99,6 +99,8 @@ return arg.beginOffset() == R.Begin && arg.endOffset() == R.End; } +MATCHER_P(PragmaTrivia, P, "") { return arg.Trivia == P; } + MATCHER(EqInc, "") { Inclusion Actual = testing::get<0>(arg); Inclusion Expected = testing::get<1>(arg); @@ -815,6 +817,27 @@ EXPECT_FALSE(mainIsGuarded(AST)); } +TEST(ParsedASTTest, DiscoversPragmaMarks) { + TestTU TU; + TU.AdditionalFiles["Header.h"] = R"( + #pragma mark - Something API + int something(); + #pragma mark Something else + )"; + TU.Code = R"cpp( + #include "Header.h" + #pragma mark In Preamble + #pragma mark - Something Impl + int something() { return 1; } + #pragma mark End + )cpp"; + auto AST = TU.build(); + + EXPECT_THAT(AST.getMarks(), ElementsAre(PragmaTrivia(" In Preamble"), + PragmaTrivia(" - Something Impl"), + PragmaTrivia(" End"))); +} + } // namespace } // namespace clangd } // namespace clang Index: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp +++ clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp @@ -42,7 +42,7 @@ MATCHER_P(SymNameRange, Range, "") { return arg.selectionRange == Range; } template <class... ChildMatchers> ::testing::Matcher<DocumentSymbol> Children(ChildMatchers... ChildrenM) { - return Field(&DocumentSymbol::children, ElementsAre(ChildrenM...)); + return Field(&DocumentSymbol::children, UnorderedElementsAre(ChildrenM...)); } std::vector<SymbolInformation> getSymbols(TestTU &TU, llvm::StringRef Query, @@ -1027,6 +1027,100 @@ 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()), + UnorderedElementsAre( + 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, PragmaMarkGroupsNesting) { + TestTU TU; + TU.ExtraArgs = {"-xobjective-c++", "-Wno-objc-root-class"}; + Annotations Main(R"cpp( + #pragma mark - Foo + struct Foo { + #pragma mark - Bar + void bar() { + #pragma mark - NotTopDecl + } + }; + void bar() {} + )cpp"); + TU.Code = Main.code().str(); + EXPECT_THAT( + getSymbols(TU.build()), + UnorderedElementsAre(AllOf( + WithName("Foo"), + Children(AllOf(WithName("Foo"), + Children(AllOf(WithName("Bar"), + Children(AllOf(WithName("bar"), + Children(WithName( + "NotTopDecl"))))))), + WithName("bar"))))); +} + +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()), + UnorderedElementsAre(WithName("Helpers"), WithName("helpA"), + WithName("(unnamed group)"), + WithName("Core"), WithName("coreMethod"))); +} + } // namespace } // namespace clangd } // namespace clang Index: clang-tools-extra/clangd/SourceCode.h =================================================================== --- clang-tools-extra/clangd/SourceCode.h +++ 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. Index: clang-tools-extra/clangd/SourceCode.cpp =================================================================== --- clang-tools-extra/clangd/SourceCode.cpp +++ 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<size_t, size_t> offsetToClangLineColumn(llvm::StringRef Code, size_t Offset) { Offset = std::min(Code.size(), Offset); Index: clang-tools-extra/clangd/Preamble.h =================================================================== --- clang-tools-extra/clangd/Preamble.h +++ 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<PragmaMark> 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<PreambleFileStatusCache> StatCache; Index: clang-tools-extra/clangd/Preamble.cpp =================================================================== --- clang-tools-extra/clangd/Preamble.cpp +++ clang-tools-extra/clangd/Preamble.cpp @@ -73,6 +73,8 @@ MainFileMacros takeMacros() { return std::move(Macros); } + std::vector<PragmaMark> takeMarks() { return std::move(Marks); } + CanonicalIncludes takeCanonicalIncludes() { return std::move(CanonIncludes); } bool isMainFileIncludeGuarded() const { return IsMainFileIncludeGuarded; } @@ -103,7 +105,9 @@ return std::make_unique<PPChainedCallbacks>( collectIncludeStructureCallback(*SourceMgr, &Includes), - std::make_unique<CollectMainFileMacros>(*SourceMgr, Macros)); + std::make_unique<PPChainedCallbacks>( + std::make_unique<CollectMainFileMacros>(*SourceMgr, Macros), + collectPragmaMarksCallback(*SourceMgr, Marks))); } CommentHandler *getCommentHandler() override { @@ -130,6 +134,7 @@ IncludeStructure Includes; CanonicalIncludes CanonIncludes; MainFileMacros Macros; + std::vector<PragmaMark> Marks; bool IsMainFileIncludeGuarded = false; std::unique_ptr<CommentHandler> IWYUHandler = nullptr; const clang::LangOptions *LangOpts = nullptr; @@ -389,6 +394,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(); Index: clang-tools-extra/clangd/ParsedAST.h =================================================================== --- clang-tools-extra/clangd/ParsedAST.h +++ 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<PragmaMark> &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<const PreambleData> Preamble, std::unique_ptr<CompilerInstance> Clang, std::unique_ptr<FrontendAction> Action, syntax::TokenBuffer Tokens, - MainFileMacros Macros, std::vector<Decl *> LocalTopLevelDecls, + MainFileMacros Macros, std::vector<PragmaMark> Marks, + std::vector<Decl *> LocalTopLevelDecls, llvm::Optional<std::vector<Diag>> 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<PragmaMark> Marks; // Data, stored after parsing. None if AST was built with a stale preamble. llvm::Optional<std::vector<Diag>> Diags; // Top-level decls inside the current file. Not that this does not include Index: clang-tools-extra/clangd/ParsedAST.cpp =================================================================== --- clang-tools-extra/clangd/ParsedAST.cpp +++ clang-tools-extra/clangd/ParsedAST.cpp @@ -449,6 +449,13 @@ std::make_unique<CollectMainFileMacros>(Clang->getSourceManager(), Macros)); + std::vector<PragmaMark> Marks; + // FIXME: We need to patch the marks for stale preambles. + 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; @@ -510,7 +517,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)); } @@ -550,6 +557,7 @@ } const MainFileMacros &ParsedAST::getMacros() const { return Macros; } +const std::vector<PragmaMark> &ParsedAST::getMarks() const { return Marks; } std::size_t ParsedAST::getUsedBytes() const { auto &AST = getASTContext(); @@ -596,12 +604,14 @@ std::unique_ptr<CompilerInstance> Clang, std::unique_ptr<FrontendAction> Action, syntax::TokenBuffer Tokens, MainFileMacros Macros, + std::vector<PragmaMark> Marks, std::vector<Decl *> LocalTopLevelDecls, llvm::Optional<std::vector<Diag>> 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<HeuristicResolver>(getASTContext()); Index: clang-tools-extra/clangd/FindSymbols.cpp =================================================================== --- clang-tools-extra/clangd/FindSymbols.cpp +++ clang-tools-extra/clangd/FindSymbols.cpp @@ -523,9 +523,135 @@ ParsedAST &AST; }; +struct PragmaMarkSymbol { + DocumentSymbol DocSym; + bool IsGroup; +}; + +/// Merge in `PragmaMarkSymbols`, sorted ascending by range, into the given +/// `DocumentSymbol` tree. +void mergePragmas(DocumentSymbol &Root, ArrayRef<PragmaMarkSymbol> Pragmas) { + while (!Pragmas.empty()) { + // We'll figure out where the Pragmas.front() should go. + PragmaMarkSymbol P = std::move(Pragmas.front()); + Pragmas = Pragmas.drop_front(); + DocumentSymbol *Cur = &Root; + while (Cur->range.contains(P.DocSym.range)) { + bool Swapped = false; + for (auto &C : Cur->children) { + // We assume at most 1 child can contain the pragma (as pragmas are on + // a single line, and children have disjoint ranges). + if (C.range.contains(P.DocSym.range)) { + Cur = &C; + Swapped = true; + break; + } + } + // Cur is the parent of P since none of the children contain P. + if (!Swapped) + break; + } + // Pragma isn't a group so we can just insert it and we are done. + if (!P.IsGroup) { + Cur->children.emplace_back(std::move(P.DocSym)); + continue; + } + // Pragma is a group, so we need to figure out where it terminates: + // - If the next Pragma is not contained in Cur, P owns all of its + // parent's children which occur after P. + // - If the next pragma is contained in Cur but actually belongs to one + // of the parent's children, we temporarily skip over it and look at + // the next pragma to decide where we end. + // - Otherwise nest all of its parent's children which occur after P but + // before the next pragma. + bool TerminatedByNextPragma = false; + for (auto &NextPragma : Pragmas) { + // If we hit a pragma outside of Cur, the rest will be outside as well. + if (!Cur->range.contains(NextPragma.DocSym.range)) + break; + + // NextPragma cannot terminate P if it is nested inside a child, look for + // the next one. + if (llvm::any_of(Cur->children, [&NextPragma](const auto &Child) { + return Child.range.contains(NextPragma.DocSym.range); + })) + continue; + + // Pragma owns all the children between P and NextPragma + auto It = llvm::partition(Cur->children, + [&P, &NextPragma](const auto &S) -> bool { + return !(P.DocSym.range < S.range && + S.range < NextPragma.DocSym.range); + }); + P.DocSym.children.assign(make_move_iterator(It), + make_move_iterator(Cur->children.end())); + Cur->children.erase(It, Cur->children.end()); + TerminatedByNextPragma = true; + break; + } + if (!TerminatedByNextPragma) { + // P is terminated by the end of current symbol, hence it owns all the + // children after P. + auto It = llvm::partition(Cur->children, [&P](const auto &S) -> bool { + return !(P.DocSym.range < S.range); + }); + P.DocSym.children.assign(make_move_iterator(It), + make_move_iterator(Cur->children.end())); + Cur->children.erase(It, Cur->children.end()); + } + // Update the range for P to cover children and append to Cur. + for (DocumentSymbol &Sym : P.DocSym.children) + unionRanges(P.DocSym.range, Sym.range); + Cur->children.emplace_back(std::move(P.DocSym)); + } +} + +PragmaMarkSymbol markToSymbol(const PragmaMark &P) { + StringRef Name = StringRef(P.Trivia).trim(); + bool IsGroup = false; + // "-\s+<group name>" or "<name>" after an initial trim. The former is + // considered a group, the latter just a mark. Like Xcode, we don't consider + // `-Foo` to be a group (space(s) after the `-` is required). + // + // We need to include a name here, otherwise editors won't properly render the + // symbol. + 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)"; + } + DocumentSymbol Sym; + Sym.name = Name.str(); + Sym.kind = SymbolKind::File; + Sym.range = P.Rng; + Sym.selectionRange = P.Rng; + return {Sym, IsGroup}; +} + std::vector<DocumentSymbol> collectDocSymbols(ParsedAST &AST) { - return DocumentOutline(AST).build(); + std::vector<DocumentSymbol> Syms = DocumentOutline(AST).build(); + + const auto &PragmaMarks = AST.getMarks(); + if (PragmaMarks.empty()) + return Syms; + + std::vector<PragmaMarkSymbol> Pragmas; + Pragmas.reserve(PragmaMarks.size()); + for (const auto &P : PragmaMarks) + Pragmas.push_back(markToSymbol(P)); + Range EntireFile = { + {0, 0}, + {std::numeric_limits<int>::max(), std::numeric_limits<int>::max()}}; + DocumentSymbol Root; + Root.children = std::move(Syms); + Root.range = EntireFile; + mergePragmas(Root, llvm::makeArrayRef(Pragmas)); + return Root.children; } + } // namespace llvm::Expected<std::vector<DocumentSymbol>> getDocumentSymbols(ParsedAST &AST) { Index: clang-tools-extra/clangd/CollectMacros.h =================================================================== --- clang-tools-extra/clangd/CollectMacros.h +++ clang-tools-extra/clangd/CollectMacros.h @@ -99,6 +99,18 @@ MainFileMacros &Out; }; +/// Represents a `#pragma mark` in the main file. +/// +/// There can be at most one pragma mark per line. +struct PragmaMark { + Range Rng; + std::string Trivia; +}; + +/// Collect all pragma marks from the main file. +std::unique_ptr<PPCallbacks> +collectPragmaMarksCallback(const SourceManager &, std::vector<PragmaMark> &Out); + } // namespace clangd } // namespace clang Index: clang-tools-extra/clangd/CollectMacros.cpp =================================================================== --- clang-tools-extra/clangd/CollectMacros.cpp +++ clang-tools-extra/clangd/CollectMacros.cpp @@ -30,5 +30,33 @@ else Out.UnknownMacros.push_back({Range, IsDefinition}); } + +class CollectPragmaMarks : public PPCallbacks { +public: + explicit CollectPragmaMarks(const SourceManager &SM, + std::vector<clangd::PragmaMark> &Out) + : SM(SM), Out(Out) {} + + void PragmaMark(SourceLocation Loc, StringRef Trivia) override { + if (isInsideMainFile(Loc, SM)) { + // FIXME: This range should just cover `XX` in `#pragma mark XX` and + // `- XX` in `#pragma mark - XX`. + Position Start = sourceLocToPosition(SM, Loc); + Position End = {Start.line + 1, 0}; + Out.emplace_back(clangd::PragmaMark{{Start, End}, Trivia.str()}); + } + } + +private: + const SourceManager &SM; + std::vector<clangd::PragmaMark> &Out; +}; + +std::unique_ptr<PPCallbacks> +collectPragmaMarksCallback(const SourceManager &SM, + std::vector<PragmaMark> &Out) { + return std::make_unique<CollectPragmaMarks>(SM, Out); +} + } // namespace clangd } // namespace clang
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits