[PATCH] D91123: [clangd] Call hierarchy (ClangdServer layer)

[Nathan Ridge via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4cb976e014db: [clangd] Call hierarchy (ClangdServer layer) 
(authored by nridge).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91123/new/

https://reviews.llvm.org/D91123

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h


Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -242,6 +242,14 @@
 TypeHierarchyDirection Direction,
 Callback> CB);
 
+  /// Get information about call hierarchy for a given position.
+  void prepareCallHierarchy(PathRef File, Position Pos,
+Callback> CB);
+
+  /// Resolve incoming calls for a given call hierarchy item.
+  void incomingCalls(const CallHierarchyItem ,
+ Callback>);
+
   /// Retrieve the top symbols from the workspace matching a query.
   void workspaceSymbols(StringRef Query, int Limit,
 Callback> CB);
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -678,6 +678,26 @@
   CB(Item);
 }
 
+void ClangdServer::prepareCallHierarchy(
+PathRef File, Position Pos, Callback> CB) {
+  auto Action = [File = File.str(), Pos,
+ CB = std::move(CB)](Expected InpAST) mutable {
+if (!InpAST)
+  return CB(InpAST.takeError());
+CB(clangd::prepareCallHierarchy(InpAST->AST, Pos, File));
+  };
+  WorkScheduler.runWithAST("Call Hierarchy", File, std::move(Action));
+}
+
+void ClangdServer::incomingCalls(
+const CallHierarchyItem ,
+Callback> CB) {
+  WorkScheduler.run("Incoming Calls", "",
+[CB = std::move(CB), Item, this]() mutable {
+  CB(clangd::incomingCalls(Item, Index));
+});
+}
+
 void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams ) {
   // FIXME: Do nothing for now. This will be used for indexing and potentially
   // invalidating other caches.


Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -242,6 +242,14 @@
 TypeHierarchyDirection Direction,
 Callback> CB);
 
+  /// Get information about call hierarchy for a given position.
+  void prepareCallHierarchy(PathRef File, Position Pos,
+Callback> CB);
+
+  /// Resolve incoming calls for a given call hierarchy item.
+  void incomingCalls(const CallHierarchyItem ,
+ Callback>);
+
   /// Retrieve the top symbols from the workspace matching a query.
   void workspaceSymbols(StringRef Query, int Limit,
 Callback> CB);
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -678,6 +678,26 @@
   CB(Item);
 }
 
+void ClangdServer::prepareCallHierarchy(
+PathRef File, Position Pos, Callback> CB) {
+  auto Action = [File = File.str(), Pos,
+ CB = std::move(CB)](Expected InpAST) mutable {
+if (!InpAST)
+  return CB(InpAST.takeError());
+CB(clangd::prepareCallHierarchy(InpAST->AST, Pos, File));
+  };
+  WorkScheduler.runWithAST("Call Hierarchy", File, std::move(Action));
+}
+
+void ClangdServer::incomingCalls(
+const CallHierarchyItem ,
+Callback> CB) {
+  WorkScheduler.run("Incoming Calls", "",
+[CB = std::move(CB), Item, this]() mutable {
+  CB(clangd::incomingCalls(Item, Index));
+});
+}
+
 void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams ) {
   // FIXME: Do nothing for now. This will be used for indexing and potentially
   // invalidating other caches.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91123: [clangd] Call hierarchy (ClangdServer layer)

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 306953.
nridge added a comment.

Update as per API changes in xrefs patch


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91123/new/

https://reviews.llvm.org/D91123

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h


Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -242,6 +242,14 @@
 TypeHierarchyDirection Direction,
 Callback> CB);
 
+  /// Get information about call hierarchy for a given position.
+  void prepareCallHierarchy(PathRef File, Position Pos,
+Callback> CB);
+
+  /// Resolve incoming calls for a given call hierarchy item.
+  void incomingCalls(const CallHierarchyItem ,
+ Callback>);
+
   /// Retrieve the top symbols from the workspace matching a query.
   void workspaceSymbols(StringRef Query, int Limit,
 Callback> CB);
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -677,6 +677,26 @@
   CB(Item);
 }
 
+void ClangdServer::prepareCallHierarchy(
+PathRef File, Position Pos, Callback> CB) {
+  auto Action = [File = File.str(), Pos,
+ CB = std::move(CB)](Expected InpAST) mutable {
+if (!InpAST)
+  return CB(InpAST.takeError());
+CB(clangd::prepareCallHierarchy(InpAST->AST, Pos, File));
+  };
+  WorkScheduler.runWithAST("Call Hierarchy", File, std::move(Action));
+}
+
+void ClangdServer::incomingCalls(
+const CallHierarchyItem ,
+Callback> CB) {
+  WorkScheduler.run("Incoming Calls", "",
+[CB = std::move(CB), Item, this]() mutable {
+  CB(clangd::incomingCalls(Item, Index));
+});
+}
+
 void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams ) {
   // FIXME: Do nothing for now. This will be used for indexing and potentially
   // invalidating other caches.


Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -242,6 +242,14 @@
 TypeHierarchyDirection Direction,
 Callback> CB);
 
+  /// Get information about call hierarchy for a given position.
+  void prepareCallHierarchy(PathRef File, Position Pos,
+Callback> CB);
+
+  /// Resolve incoming calls for a given call hierarchy item.
+  void incomingCalls(const CallHierarchyItem ,
+ Callback>);
+
   /// Retrieve the top symbols from the workspace matching a query.
   void workspaceSymbols(StringRef Query, int Limit,
 Callback> CB);
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -677,6 +677,26 @@
   CB(Item);
 }
 
+void ClangdServer::prepareCallHierarchy(
+PathRef File, Position Pos, Callback> CB) {
+  auto Action = [File = File.str(), Pos,
+ CB = std::move(CB)](Expected InpAST) mutable {
+if (!InpAST)
+  return CB(InpAST.takeError());
+CB(clangd::prepareCallHierarchy(InpAST->AST, Pos, File));
+  };
+  WorkScheduler.runWithAST("Call Hierarchy", File, std::move(Action));
+}
+
+void ClangdServer::incomingCalls(
+const CallHierarchyItem ,
+Callback> CB) {
+  WorkScheduler.run("Incoming Calls", "",
+[CB = std::move(CB), Item, this]() mutable {
+  CB(clangd::incomingCalls(Item, Index));
+});
+}
+
 void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams ) {
   // FIXME: Do nothing for now. This will be used for indexing and potentially
   // invalidating other caches.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91123: [clangd] Call hierarchy (ClangdServer layer)

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet accepted this revision.
kadircet added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:695
+Callback>> CB) {
+  CB(clangd::incomingCalls(Item, Index));
+}

nridge wrote:
> kadircet wrote:
> > why do we run this on the mainthread ? I suppose we should just do 
> > `WorkScheduler.run`
> I was following what `resolveTypeHierarchy` did. Should I change that too?
yes, i think we should. preferably on a different patch though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91123/new/

https://reviews.llvm.org/D91123

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91123: [clangd] Call hierarchy (ClangdServer layer)

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

In D91123#2387032 , @kadircet wrote:

> can you also add some tests to ClangdTests.cpp ?

It seems like they would be highly duplicative of the tests in 
CallHierarchyTests.cpp.




Comment at: clang-tools-extra/clangd/ClangdServer.cpp:695
+Callback>> CB) {
+  CB(clangd::incomingCalls(Item, Index));
+}

kadircet wrote:
> why do we run this on the mainthread ? I suppose we should just do 
> `WorkScheduler.run`
I was following what `resolveTypeHierarchy` did. Should I change that too?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91123/new/

https://reviews.llvm.org/D91123

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91123: [clangd] Call hierarchy (ClangdServer layer)

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 305408.
nridge marked 2 inline comments as done.
nridge added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91123/new/

https://reviews.llvm.org/D91123

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h


Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -242,6 +242,16 @@
 TypeHierarchyDirection Direction,
 Callback> CB);
 
+  /// Get information about call hierarchy for a given position.
+  void prepareCallHierarchy(
+  PathRef File, Position Pos,
+  Callback>> CB);
+
+  /// Resolve incoming calls for a given call hierarchy item.
+  void incomingCalls(
+  const CallHierarchyItem ,
+  Callback>>);
+
   /// Retrieve the top symbols from the workspace matching a query.
   void workspaceSymbols(StringRef Query, int Limit,
 Callback> CB);
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -677,6 +677,27 @@
   CB(Item);
 }
 
+void ClangdServer::prepareCallHierarchy(
+PathRef File, Position Pos,
+Callback>> CB) {
+  auto Action = [File = File.str(), Pos,
+ CB = std::move(CB)](Expected InpAST) mutable {
+if (!InpAST)
+  return CB(InpAST.takeError());
+CB(clangd::prepareCallHierarchy(InpAST->AST, Pos, File));
+  };
+  WorkScheduler.runWithAST("Call Hierarchy", File, std::move(Action));
+}
+
+void ClangdServer::incomingCalls(
+const CallHierarchyItem ,
+Callback>> CB) {
+  WorkScheduler.run("Incoming Calls", "",
+[CB = std::move(CB), Item, this]() mutable {
+  CB(clangd::incomingCalls(Item, Index));
+});
+}
+
 void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams ) {
   // FIXME: Do nothing for now. This will be used for indexing and potentially
   // invalidating other caches.


Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -242,6 +242,16 @@
 TypeHierarchyDirection Direction,
 Callback> CB);
 
+  /// Get information about call hierarchy for a given position.
+  void prepareCallHierarchy(
+  PathRef File, Position Pos,
+  Callback>> CB);
+
+  /// Resolve incoming calls for a given call hierarchy item.
+  void incomingCalls(
+  const CallHierarchyItem ,
+  Callback>>);
+
   /// Retrieve the top symbols from the workspace matching a query.
   void workspaceSymbols(StringRef Query, int Limit,
 Callback> CB);
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -677,6 +677,27 @@
   CB(Item);
 }
 
+void ClangdServer::prepareCallHierarchy(
+PathRef File, Position Pos,
+Callback>> CB) {
+  auto Action = [File = File.str(), Pos,
+ CB = std::move(CB)](Expected InpAST) mutable {
+if (!InpAST)
+  return CB(InpAST.takeError());
+CB(clangd::prepareCallHierarchy(InpAST->AST, Pos, File));
+  };
+  WorkScheduler.runWithAST("Call Hierarchy", File, std::move(Action));
+}
+
+void ClangdServer::incomingCalls(
+const CallHierarchyItem ,
+Callback>> CB) {
+  WorkScheduler.run("Incoming Calls", "",
+[CB = std::move(CB), Item, this]() mutable {
+  CB(clangd::incomingCalls(Item, Index));
+});
+}
+
 void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams ) {
   // FIXME: Do nothing for now. This will be used for indexing and potentially
   // invalidating other caches.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91123: [clangd] Call hierarchy (ClangdServer layer)

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

can you also add some tests to ClangdTests.cpp ?




Comment at: clang-tools-extra/clangd/ClangdServer.cpp:695
+Callback>> CB) {
+  CB(clangd::incomingCalls(Item, Index));
+}

why do we run this on the mainthread ? I suppose we should just do 
`WorkScheduler.run`



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:701
+Callback>> CB) {
+  CB(clangd::outgoingCalls(Item, Index));
+}

can we reply with none/empty at clangdlsplayer directly and delete the 
`outgoingCalls` from both clangdserver and xrefs.cpp ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91123/new/

https://reviews.llvm.org/D91123

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91123: [clangd] Call hierarchy (ClangdServer layer)

[Nathan Ridge via Phabricator via cfe-commits]

nridge created this revision.
nridge added a reviewer: kadircet.
Herald added subscribers: cfe-commits, usaxena95, arphaman.
Herald added a project: clang.
nridge requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91123

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h


Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -242,6 +242,21 @@
 TypeHierarchyDirection Direction,
 Callback> CB);
 
+  /// Get information about call hierarchy for a given position.
+  void prepareCallHierarchy(
+  PathRef File, Position Pos,
+  Callback>> CB);
+
+  /// Resolve incoming calls for a given call hierarchy item.
+  void incomingCalls(
+  const CallHierarchyItem ,
+  Callback>>);
+
+  /// Resolve outgoing calls for a given call hierarchy item.
+  void outgoingCalls(
+  const CallHierarchyItem ,
+  Callback>>);
+
   /// Retrieve the top symbols from the workspace matching a query.
   void workspaceSymbols(StringRef Query, int Limit,
 Callback> CB);
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -677,6 +677,30 @@
   CB(Item);
 }
 
+void ClangdServer::prepareCallHierarchy(
+PathRef File, Position Pos,
+Callback>> CB) {
+  auto Action = [File = File.str(), Pos, CB = std::move(CB),
+ this](Expected InpAST) mutable {
+if (!InpAST)
+  return CB(InpAST.takeError());
+CB(clangd::prepareCallHierarchy(InpAST->AST, Pos, Index, File));
+  };
+  WorkScheduler.runWithAST("Call Hierarchy", File, std::move(Action));
+}
+
+void ClangdServer::incomingCalls(
+const CallHierarchyItem ,
+Callback>> CB) {
+  CB(clangd::incomingCalls(Item, Index));
+}
+
+void ClangdServer::outgoingCalls(
+const CallHierarchyItem ,
+Callback>> CB) {
+  CB(clangd::outgoingCalls(Item, Index));
+}
+
 void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams ) {
   // FIXME: Do nothing for now. This will be used for indexing and potentially
   // invalidating other caches.


Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -242,6 +242,21 @@
 TypeHierarchyDirection Direction,
 Callback> CB);
 
+  /// Get information about call hierarchy for a given position.
+  void prepareCallHierarchy(
+  PathRef File, Position Pos,
+  Callback>> CB);
+
+  /// Resolve incoming calls for a given call hierarchy item.
+  void incomingCalls(
+  const CallHierarchyItem ,
+  Callback>>);
+
+  /// Resolve outgoing calls for a given call hierarchy item.
+  void outgoingCalls(
+  const CallHierarchyItem ,
+  Callback>>);
+
   /// Retrieve the top symbols from the workspace matching a query.
   void workspaceSymbols(StringRef Query, int Limit,
 Callback> CB);
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -677,6 +677,30 @@
   CB(Item);
 }
 
+void ClangdServer::prepareCallHierarchy(
+PathRef File, Position Pos,
+Callback>> CB) {
+  auto Action = [File = File.str(), Pos, CB = std::move(CB),
+ this](Expected InpAST) mutable {
+if (!InpAST)
+  return CB(InpAST.takeError());
+CB(clangd::prepareCallHierarchy(InpAST->AST, Pos, Index, File));
+  };
+  WorkScheduler.runWithAST("Call Hierarchy", File, std::move(Action));
+}
+
+void ClangdServer::incomingCalls(
+const CallHierarchyItem ,
+Callback>> CB) {
+  CB(clangd::incomingCalls(Item, Index));
+}
+
+void ClangdServer::outgoingCalls(
+const CallHierarchyItem ,
+Callback>> CB) {
+  CB(clangd::outgoingCalls(Item, Index));
+}
+
 void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams ) {
   // FIXME: Do nothing for now. This will be used for indexing and potentially
   // invalidating other caches.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits