Skip to content

Call hierarchy support#1955

Merged
pepeiborra merged 33 commits into
haskell:masterfrom
July541:call-hierarchy
Jul 27, 2021
Merged

Call hierarchy support#1955
pepeiborra merged 33 commits into
haskell:masterfrom
July541:call-hierarchy

Conversation

@July541

@July541 July541 commented Jun 21, 2021

Copy link
Copy Markdown
Collaborator

Hello all,

This pr is about my GSoC 2021 project, call hierarchy support. I create a draft to show my progress to those who may be interested.

Progress

  • Prepare call hierarchy (working now)
  • Incoming calls
  • Outgoing calls

Known issues

  • Data family range error (see commented test)
  • Incoming/Outgoing test missing
  • Name display not optimized for DuplicateRecordFields

@July541
July541 marked this pull request as draft June 21, 2021 13:01
@wz1000

wz1000 commented Jun 21, 2021

Copy link
Copy Markdown
Collaborator

If I understand correctly, call hierarchy is meant to be global and project wide. However, you seem to be generating the tree using a single HieAst.

I think it would be a good idea to make use of hiedb for this rather than directly using .hie files/HieAst. Take a look at the getGraph, getReachable and getVertices functions in hiedb: https://github.com/wz1000/HieDb/blob/master/src/HieDb/Query.hs#L175

The code you have for traversing the HieAst could still be very useful - we only keep up to date version of files the user isn't currently editing in the hiedb. For the files the user has open in their editor (IsFileOfInterest => True), to get an accurate callgraph the information from the HieAst traversal would need to be combined with the information from hiedb.

@July541

July541 commented Jun 21, 2021

Copy link
Copy Markdown
Collaborator Author

If I understand correctly, call hierarchy is meant to be global and project wide. However, you seem to be generating the tree using a single HieAst.

You're right, currently, I just finished the part of prepare call hierarchy. This part only constructs the CallHierarchyItem without any relation to other statements. I think HieAst is enough now.

The hierarchy is queried at the incoming/outgoing steps, which I have not started, your suggestion is very feasible, I'll follow it in the next part.

@pepeiborra

Copy link
Copy Markdown
Collaborator

@July541 I can see that you uploaded some commits yesterday. Care to share an update on what the recent changes are?

@July541

July541 commented Jul 8, 2021

Copy link
Copy Markdown
Collaborator Author

@pepeiborra It's about a prototype of the implementation of outgoing calls which has functionality but a bad code style. I'll show a complete call hierarchy after incoming calls are finished, it'll come soon.

@July541

July541 commented Jul 11, 2021

Copy link
Copy Markdown
Collaborator Author
2021-07-11.21-45-42.mp4

Recent progress, incoming calls and outgoing calls have basic functions, see demo above for details.

Current problems:

  • Duplication items(multi range merge)
  • Range of incoming/outgoing calls still exist problems
  • Test missing
  • Query will be broken if the target is a parameter

@pepeiborra

Copy link
Copy Markdown
Collaborator

This looks quite reasonable to me, nice job @July541 !
I would argue for landing it and then opening new tickets to fix the outstanding problems.
What do you think ?

import Name

incomingCalls :: HieDb -> Symbol -> IO [Vertex]
incomingCalls (getConn -> conn) symbol = do

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It would be best to move these functions into hiedb.

@July541

July541 commented Jul 12, 2021

Copy link
Copy Markdown
Collaborator Author

@pepeiborra I agree with you. I think it can be reviewed after the break issue is fixed, not take long.

@wz1000 It turns out I wrote it in HieDb from the beginning. But the following questions bothered me and made me turn to the internal implementation:

  1. The definition of Vertex is vague, we may need to rewrite the relevant parts, in which the compatibility comes at a high price to keep code elegant(for HieDb developer, not user).

  2. The two calls(incoming/outgoing) seem to be domain-specific, and I'm not sure they will have other usages, so I wrote them inside the call hierarchy.

Honestly, I'm not sure it's a reasonable solution. if you think that move to HieDb is better, I'll transfer it.

@pepeiborra

Copy link
Copy Markdown
Collaborator

@pepeiborra I agree with you. I think it can be reviewed after the break issue is fixed, not take long.

Awesome, please shout if you need any help

@isovector

Copy link
Copy Markdown
Collaborator

Your CI appears to be blocked by #2016

@July541
July541 marked this pull request as ready for review July 15, 2021 10:49
Comment thread .github/workflows/test.yml Outdated
Comment thread plugins/hls-call-hierarchy-plugin/src/Ide/Plugin/CallHierarchy/Query.hs Outdated
Comment thread plugins/hls-call-hierarchy-plugin/src/Ide/Plugin/CallHierarchy/Query.hs Outdated
@July541

July541 commented Jul 27, 2021

Copy link
Copy Markdown
Collaborator Author

I think the current fail can be fixed after the new lsp is released.

@pepeiborra

Copy link
Copy Markdown
Collaborator

This looks ready to merge, great job @July541 !!!
Do you want to do the honours @jneira ?

@jneira

jneira commented Jul 27, 2021

Copy link
Copy Markdown
Member

@July541 amazing work, hls users will be amazed, congrats
And thank you @pepeiborra for taking care, I had not all time I wish

@pepeiborra
pepeiborra merged commit d815d04 into haskell:master Jul 27, 2021
@July541

July541 commented Jul 28, 2021

Copy link
Copy Markdown
Collaborator Author

@pepeiborra I noticed an unexpected ghcide fail on 8.6.5, not sure if it is caused by me.

@jhrcek

jhrcek commented Jul 28, 2021

Copy link
Copy Markdown
Collaborator

I pulled master this morning and I'm also not able to do stack ./install.hs hls-8.8.4 locally.
It seems it's related to this PR.
I'm getting build failures similar to what I see in recent CI runs:
e.g. this

build failures in CI
Linking .stack-work/dist/x86_64-linux/Cabal-3.0.1.0/build/ghcide/ghcide ...
ghcide                           > /root/.stack/snapshots/x86_64-linux/26f5581bf7f9f176d3f7ba334912f1d717717a1ecee407868101798f77c5c139/8.8.3/lib/x86_64-linux-ghc-8.8.3/libHSlsp-1.2.0.0-LiIFgQ1fCTv73zSCj7uRmO-ghc8.8.3.so: error: undefined reference to 'lspzmtypeszm1zi2zi0zi0zmDHDZZnuOdhmBHZZjePkaCNbQ_LanguageziLSPziTypesziCallHierarchy_zdfFromJSONCallHierarchyOutgoingCallsParams_closure'
ghcide                           > /root/.stack/snapshots/x86_64-linux/26f5581bf7f9f176d3f7ba334912f1d717717a1ecee407868101798f77c5c139/8.8.3/lib/x86_64-linux-ghc-8.8.3/libHSlsp-1.2.0.0-LiIFgQ1fCTv73zSCj7uRmO-ghc8.8.3.so: error: undefined reference to 'lspzmtypeszm1zi2zi0zi0zmDHDZZnuOdhmBHZZjePkaCNbQ_LanguageziLSPziTypesziCallHierarchy_zdfFromJSONCallHierarchyIncomingCallsParams_closure'
ghcide                           > /root/.stack/snapshots/x86_64-linux/26f5581bf7f9f176d3f7ba334912f1d717717a1ecee407868101798f77c5c139/8.8.3/lib/x86_64-linux-ghc-8.8.3/libHSlsp-1.2.0.0-LiIFgQ1fCTv73zSCj7uRmO-ghc8.8.3.so: error: undefined reference to 'lspzmtypeszm1zi2zi0zi0zmDHDZZnuOdhmBHZZjePkaCNbQ_LanguageziLSPziTypesziCallHierarchy_zdfFromJSONCallHierarchyPrepareParams_closure'
ghcide                           > /root/.stack/snapshots/x86_64-linux/26f5581bf7f9f176d3f7ba334912f1d717717a1ecee407868101798f77c5c139/8.8.3/lib/x86_64-linux-ghc-8.8.3/libHSlsp-1.2.0.0-LiIFgQ1fCTv73zSCj7uRmO-ghc8.8.3.so: error: undefined reference to 'lspzmtypeszm1zi2zi0zi0zmDHDZZnuOdhmBHZZjePkaCNbQ_LanguageziLSPziTypesziMethod_zdfFromJSONSMethod54_closure'
ghcide                           > /root/.stack/snapshots/x86_64-linux/26f5581bf7f9f176d3f7ba334912f1d717717a1ecee407868101798f77c5c139/8.8.3/lib/x86_64-linux-ghc-8.8.3/libHSlsp-1.2.0.0-LiIFgQ1fCTv73zSCj7uRmO-ghc8.8.3.so: error: undefined reference to 'lspzmtypeszm1zi2zi0zi0zmDHDZZnuOdhmBHZZjePkaCNbQ_LanguageziLSPziTypesziMethod_zdfFromJSONSMethod55_closure'
ghcide                           > /root/.stack/snapshots/x86_64-linux/26f5581bf7f9f176d3f7ba334912f1d717717a1ecee407868101798f77c5c139/8.8.3/lib/x86_64-linux-ghc-8.8.3/libHSlsp-1.2.0.0-LiIFgQ1fCTv73zSCj7uRmO-ghc8.8.3.so: error: undefined reference to 'lspzmtypeszm1zi2zi0zi0zmDHDZZnuOdhmBHZZjePkaCNbQ_LanguageziLSPziTypesziMethod_zdWSTextDocumentPrepareCallHierarchy_closure'
ghcide                           > /root/build/ghcide/.stack-work/dist/x86_64-linux/Cabal-3.0.1.0/build/libHSghcide-1.4.0.3-GzSd9eXvctl5jyqcFJIb49-ghc8.8.3.so: error: undefined reference to 'lspzmtypeszm1zi2zi0zi0zmDHDZZnuOdhmBHZZjePkaCNbQ_LanguageziLSPziTypesziMethod_zdfShowSMethod114_closure'
ghcide                           > /root/build/ghcide/.stack-work/dist/x86_64-linux/Cabal-3.0.1.0/build/libHSghcide-1.4.0.3-GzSd9eXvctl5jyqcFJIb49-ghc8.8.3.so: error: undefined reference to 'lspzmtypeszm1zi2zi0zi0zmDHDZZnuOdhmBHZZjePkaCNbQ_LanguageziLSPziTypesziMethod_zdfShowSMethod112_closure'
ghcide                           > /root/build/ghcide/.stack-work/dist/x86_64-linux/Cabal-3.0.1.0/build/libHSghcide-1.4.0.3-GzSd9eXvctl5jyqcFJIb49-ghc8.8.3.so: error: undefined reference to 'lspzmtypeszm1zi2zi0zi0zmDHDZZnuOdhmBHZZjePkaCNbQ_LanguageziLSPziTypesziMethod_zdfShowSMethod116_closure'
ghcide                           > collect2: error: ld returned 1 exit status
ghcide                           > `gcc' failed in phase `Linker'. (Exit code: 1)
  

see failure in CI

@pepeiborra

pepeiborra commented Jul 28, 2021

Copy link
Copy Markdown
Collaborator

Have you tried stack clean ?

@pepeiborra

Copy link
Copy Markdown
Collaborator

@pepeiborra I noticed an unexpected ghcide fail on 8.6.5, not sure if it is caused by me.

Some tests are flaky in Windows, that's not caused by this PR

@July541

July541 commented Jul 28, 2021

Copy link
Copy Markdown
Collaborator Author

@jhrcek Ideally the updated dependencies are OK for the build.

- github: haskell/lsp
commit: ef59c28b41ed4c5775f0ab0c1e985839359cec96
subdirs:
- lsp-types
- lsp
- lsp-test
# https://github.com/haskell/lsp/pull/332

@jneira

jneira commented Jul 28, 2021

Copy link
Copy Markdown
Member

@jhrcek i hope #2044 (which remove .stack-work from cache, so equivalent to do a stack clean --full) will take ride of those circleci linker errors

@pepeiborra

Copy link
Copy Markdown
Collaborator

@jhrcek i hope #2044 (which remove .stack-work from cache, so equivalent to do a stack clean --full) will take ride of those circleci linker errors

Users will still be affected by the stack build problem though.

@jhrcek

jhrcek commented Jul 28, 2021

Copy link
Copy Markdown
Collaborator

I tried stack clean --full in haskell-language-server repo, but still got build failure during linking.
In the end I removed all build snapshots from ~/.stack/snapshots/x86_64-linux-tinfo6/ and then it built correctly.
The call hierarchy seems to be working great. Thank you! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants