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

Refinement holes #748

Merged
merged 5 commits into from
Sep 5, 2020
Merged

Refinement holes #748

merged 5 commits into from
Sep 5, 2020

Conversation

pepeiborra
Copy link
Collaborator

Fixes #639

@pepeiborra pepeiborra force-pushed the typed-holes branch 6 times, most recently from e8cd500 to 7790be9 Compare September 2, 2020 07:53
@pepeiborra pepeiborra marked this pull request as draft September 3, 2020 08:07
@pepeiborra
Copy link
Collaborator Author

This change needs a bit more work, as I found some more complex examples of refinement hole fits are not handled by the new regex approach

@pepeiborra pepeiborra force-pushed the typed-holes branch 2 times, most recently from b415635 to 38f3afb Compare September 4, 2020 14:35
@pepeiborra pepeiborra marked this pull request as ready for review September 4, 2020 14:36
@pepeiborra pepeiborra force-pushed the typed-holes branch 2 times, most recently from 2ffa9cf to 53a5d9a Compare September 4, 2020 14:53
@pepeiborra
Copy link
Collaborator Author

pepeiborra commented Sep 4, 2020

My second pass is ready for re-review:

  • Set a number of GHC flags that make parsing the diagnostics much simpler, ensure consistent behaviour and improve sorting
  • Simplified the parsing code
  • Sorted the code actions by priority, to prevent long lists of typed holes from burying other suggestions
  • Add a hook to customise the DynFlags in case we want to make the typed hole settings available to the user. Refinement holes are very powerful but make type checking an order of magnitude slower (thankfully, this only applies when the program contains holes)

Refinement hole fits are very cool, but currently too slow to enable at deeper
levels. It should eventually be user configurable.
@pepeiborra
Copy link
Collaborator Author

/azp run

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@pepeiborra pepeiborra force-pushed the typed-holes branch 2 times, most recently from bff6195 to 4edf523 Compare September 5, 2020 04:35
@wz1000
Copy link
Collaborator

wz1000 commented Sep 5, 2020

Is this OK to merge?

@pepeiborra
Copy link
Collaborator Author

Yes, good to merge if changes requested after re-review

@@ -94,6 +94,9 @@ data IdeOptions = IdeOptions
-- Otherwise, return the result of parsing without Opt_Haddock, so
-- that the parsed module contains the result of Opt_KeepRawTokenStream,
-- which might be necessary for hlint.
, optCustomDynFlags :: DynFlags -> DynFlags
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this for hls and other downstream users of ghcide?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep

@wz1000 wz1000 merged commit 8f0a4f8 into haskell:master Sep 5, 2020
$ flip gopt_unset Opt_ShowTypeAppOfHoleFits -- not used
$ flip gopt_unset Opt_ShowTypeAppVarsOfHoleFits -- not used
$ flip gopt_unset Opt_ShowTypeOfHoleFits -- massively simplifies parsing
$ flip gopt_set Opt_SortBySubsumHoleFits -- very nice and fast enough in most cases
Copy link

Choose a reason for hiding this comment

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

This line slows the whole thing by a lot, since it does pairwise comparison between every single possible fit, and everything grinds to a halt. I'd strongly recommend you turn it off!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You mean sort by subsumption right? It wasn't too bad in my empirical tests. It's quadratic complexity but the number of items is small, so only in degenerate cases like _ :: a does it grind to a halt, and ghcide will cancel it as soon as an edit is made anyway.

Overall, there is no set of flags that will make everyone happy, so we should eventually expose this flags as a user configurable setting.

pepeiborra added a commit to pepeiborra/ide that referenced this pull request Dec 29, 2020
* Refinement holes

* Set more GHC options and use indentation for parsing

* Add an option to customize the typed holes settings

Refinement hole fits are very cool, but currently too slow to enable at deeper
levels. It should eventually be user configurable.

* GHC Compatibility

* Compat. with 8.4
pepeiborra added a commit to pepeiborra/ide that referenced this pull request Dec 29, 2020
* Refinement holes

* Set more GHC options and use indentation for parsing

* Add an option to customize the typed holes settings

Refinement hole fits are very cool, but currently too slow to enable at deeper
levels. It should eventually be user configurable.

* GHC Compatibility

* Compat. with 8.4
pepeiborra added a commit to pepeiborra/ide that referenced this pull request Dec 29, 2020
* Refinement holes

* Set more GHC options and use indentation for parsing

* Add an option to customize the typed holes settings

Refinement hole fits are very cool, but currently too slow to enable at deeper
levels. It should eventually be user configurable.

* GHC Compatibility

* Compat. with 8.4
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.

Improved fill hole suggestions
4 participants