Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cache instances from call hierarchy provider #10857

Merged
merged 9 commits into from
Apr 25, 2022

Conversation

colin-grant-work
Copy link
Contributor

@colin-grant-work colin-grant-work commented Mar 7, 2022

What it does

As part of #10310, it was found that Theia does not satisfy the expectations of VSCode CLangD (or any other language client based on vscode-languageclient) because we do not guarantee that the plugin will receive the same instances of CallHierarchyItems that it creates when calls are made.

This PR addresses that problem by caching call hierarchy items received from plugins and retrieving them when passing them back into the plugin.

In addition, it moves some way to addressing #10528, but with a twist. Since call hierarchy items are now being cached, there's need for a signal to clear the cache, and the return from CallHierarchyAdapter.prepareCallHierarchy is now a CallHierarchySession that includes a disposable to dispose of the cache. That isn't part of the API available to plugins, so the VSCode references view never disposes of its sessions (as far as I can tell, CallHierarchySessions are never disposed of in VSCode), but our internal view does know when to dispose of its sessions, so I have left the view in, but adjusted the types to eliminate our purely internal types in favor of close mirrors of the plugin types.

How to test

  1. Install the latest CLangD plugin

To install the CLangD plugin, add this line to the .theiaPlugins field of the repo's package.json:

"vscode-clangd": "https://open-vsx.org/api/llvm-vs-code-extensions/vscode-clangd/0.1.13/file/llvm-vs-code-extensions.vscode-clangd-0.1.13.vsix"
  1. Open the examples/cpp-debug-workspace from the Theia CPP Extensions repo
  2. Open a.cpp
  3. Use Calls: Show Call Hierarchy on one of the functions there other than main.
  4. You should get results, and you should be able to expand those results.
  5. Call hierarchy for languages like TS, etc. should continue to work as before in both references view and internal view.

Review checklist

Reminder for reviewers

@colin-grant-work colin-grant-work added the call-hierarchy issues related to the call-hierarachy label Mar 7, 2022
@colin-grant-work colin-grant-work force-pushed the bugfix/CH-ensure-same-item-instance branch from 1995457 to 5fb525f Compare March 16, 2022 23:58
@colin-grant-work colin-grant-work marked this pull request as ready for review April 14, 2022 15:28
@vince-fugnitto vince-fugnitto self-requested a review April 21, 2022 18:52
@vince-fugnitto vince-fugnitto added the vscode issues related to VSCode compatibility label Apr 22, 2022
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirmed that it works well 👍

  • the code looks good.
  • confirmed that call-hierarchy from out custom view works well using clangd.
  • confirmed that show call hierarchy (plugin side) works well as well.

@colin-grant-work colin-grant-work merged commit cbc30a3 into master Apr 25, 2022
@colin-grant-work colin-grant-work deleted the bugfix/CH-ensure-same-item-instance branch April 25, 2022 19:19
@github-actions github-actions bot added this to the 1.25.0 milestone Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
call-hierarchy issues related to the call-hierarachy vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants