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

Reply via email to