Skip to content
This repository has been archived by the owner on Jan 2, 2021. It is now read-only.

Use a global namecache to read .hie files #677

Merged
merged 5 commits into from
Jul 9, 2020

Conversation

wz1000
Copy link
Collaborator

@wz1000 wz1000 commented Jun 30, 2020

This backports this GHC MR in preparation for a larger role for .hie files in ghcide.

We now have a global shared NameCache referenced by ShakeExtras, which we use to read .hie files.

@wz1000 wz1000 force-pushed the hiefile-namecache branch 3 times, most recently from 8b2570e to b260708 Compare July 1, 2020 07:50
Copy link
Collaborator

@cocreature cocreature left a comment

Choose a reason for hiding this comment

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

Nice, thank you!

@@ -0,0 +1,394 @@
{-
Copy link
Collaborator

Choose a reason for hiding this comment

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

I’m increasingly starting to wonder if GHC is the right location for the .hie files code. We seem to copy paste basically everything since we need to patch it. Hopefully, things will stabilize at some point 😞

Copy link
Collaborator Author

@wz1000 wz1000 Jul 6, 2020

Choose a reason for hiding this comment

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

It's used by haddock, so there isn't much of an alternative at this point.

Unfortunately this will soon become worse, as I have a patch to backport .hie files to ghc-8.6

@cocreature
Copy link
Collaborator

Needs rebasing.

wz1000 and others added 3 commits July 6, 2020 18:41
Co-authored-by: Matthew Pickering <matthewtpickering@gmail.com>
@wz1000 wz1000 marked this pull request as draft July 6, 2020 14:15
Since we started reusing `.hi` files, this exposes a bug where definitions
aren't available since a bad source span from the `.hi` file gets put into
the NameCache. We rectify by ensuring the span in the NameCache always matches
the one from the `.hie` file.

This has surfaced because an interaction between the commit which uses `.hi`
instead of retypechecking and the change to use the shared global NameCache
to read `.hie` files.
@wz1000 wz1000 marked this pull request as ready for review July 6, 2020 14:17
@wz1000
Copy link
Collaborator Author

wz1000 commented Jul 6, 2020

I found a bug introduced by this, fixed it and modified the tests to exercise this code path. See the commit message of 96d1f30 for an explanation.

@cocreature cocreature merged commit f32f666 into haskell:master Jul 9, 2020
@cocreature
Copy link
Collaborator

Thanks for the bugfix!

pepeiborra pushed a commit to pepeiborra/ide that referenced this pull request Dec 29, 2020
* Use global NameCache for reading HIE files

Co-authored-by: Matthew Pickering <matthewtpickering@gmail.com>

* ignore hlint

* redundant imports

* Use hie files as source of truth for name source spans.

Since we started reusing `.hi` files, this exposes a bug where definitions
aren't available since a bad source span from the `.hi` file gets put into
the NameCache. We rectify by ensuring the span in the NameCache always matches
the one from the `.hie` file.

This has surfaced because an interaction between the commit which uses `.hi`
instead of retypechecking and the change to use the shared global NameCache
to read `.hie` files.

* Add test for missing definitions

Co-authored-by: Matthew Pickering <matthewtpickering@gmail.com>
pepeiborra pushed a commit to pepeiborra/ide that referenced this pull request Dec 29, 2020
* Use global NameCache for reading HIE files

Co-authored-by: Matthew Pickering <matthewtpickering@gmail.com>

* ignore hlint

* redundant imports

* Use hie files as source of truth for name source spans.

Since we started reusing `.hi` files, this exposes a bug where definitions
aren't available since a bad source span from the `.hi` file gets put into
the NameCache. We rectify by ensuring the span in the NameCache always matches
the one from the `.hie` file.

This has surfaced because an interaction between the commit which uses `.hi`
instead of retypechecking and the change to use the shared global NameCache
to read `.hie` files.

* Add test for missing definitions

Co-authored-by: Matthew Pickering <matthewtpickering@gmail.com>
pepeiborra pushed a commit to pepeiborra/ide that referenced this pull request Dec 29, 2020
* Use global NameCache for reading HIE files

Co-authored-by: Matthew Pickering <matthewtpickering@gmail.com>

* ignore hlint

* redundant imports

* Use hie files as source of truth for name source spans.

Since we started reusing `.hi` files, this exposes a bug where definitions
aren't available since a bad source span from the `.hi` file gets put into
the NameCache. We rectify by ensuring the span in the NameCache always matches
the one from the `.hie` file.

This has surfaced because an interaction between the commit which uses `.hi`
instead of retypechecking and the change to use the shared global NameCache
to read `.hie` files.

* Add test for missing definitions

Co-authored-by: Matthew Pickering <matthewtpickering@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants