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.

Sign up for GitHub

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

Jump to bottom

Make adding missing constraint work in presence of 'forall' (fixes #1164) #1177

Merged
merged 8 commits into from
Jan 9, 2021
Merged

Make adding missing constraint work in presence of 'forall' (fixes #1164) #1177

merged 8 commits into from
Jan 9, 2021

Conversation

jhrcek
Copy link
Collaborator

@jhrcek jhrcek commented Jan 9, 2021 •
edited
Loading

Fixes #1164

worksWithForall

pepeiborra
pepeiborra approved these changes Jan 9, 2021
View reviewed changes
Copy link
Collaborator

@pepeiborra pepeiborra left a comment

Choose a reason for hiding this comment

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

good catch, thanks!

ghcide/test/exe/Main.hs Show resolved Hide resolved
ghcide/test/exe/Main.hs Show resolved Hide resolved
@pepeiborra pepeiborra added the merge me Label to trigger pull request merge label Jan 9, 2021
@jhrcek jhrcek force-pushed the jhrcek/fix-add-missing-constraint-for-types-with-forall branch from b65b289 to c3edf32 Compare January 9, 2021 13:43
@jhrcek
Copy link
Collaborator Author

jhrcek commented Jan 9, 2021

Given my reply above @pepeiborra I hope you don't mind reverting your change.
If you want I can refactor the tests so that test description and code action names are separated.

@jhrcek
Copy link
Collaborator Author

jhrcek commented Jan 9, 2021

Alright, I changed in the most obvious way by not reusing code action names for test description, instead adding separate description for each test.

jhrcek
jhrcek commented Jan 9, 2021
View reviewed changes
ghcide/src/Development/IDE/GHC/Compat.hs
@@ -283,3 +283,12 @@ pattern ExposePackage s a mr <- DynFlags.ExposePackage s a _ mr
#else
pattern ExposePackage s a mr = DynFlags.ExposePackage s a mr
#endif

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Seems like splitLHsForAllTy no longer exists since ghc 8.10 and there's now just splitLHsForAllTyInvis.
Is this way of handling this situation ok?

@mergify mergify bot merged commit 54be003 into haskell:master Jan 9, 2021
@jhrcek jhrcek deleted the jhrcek/fix-add-missing-constraint-for-types-with-forall branch January 19, 2024 04:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pepeiborra pepeiborra pepeiborra approved these changes

Assignees
No one assigned
Labels
merge me Label to trigger pull request merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Adding missing constraint to type signature is broken when "forall" is present
2 participants
@jhrcek @pepeiborra