Skip to content

Eval plugin for GHC 9.4 #3391

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
Merged
merged 1 commit into from
Feb 22, 2023
Merged

Eval plugin for GHC 9.4 #3391

merged 1 commit into from
Feb 22, 2023

Conversation

@wz1000
Copy link
Collaborator

@wz1000 wz1000 commented Dec 9, 2022

Supersedes #3249 and depends on #3390

@wz1000 wz1000 requested a review from pepeiborra as a code owner December 9, 2022 14:14
@wz1000 wz1000 mentioned this pull request Dec 14, 2022
Closed
@michaelpj
Copy link
Collaborator

Looks like Pepe's question never got answered (#3249 (comment)), but I'm okay with this anyway. Has conflicts now.

@wz1000 wz1000 mentioned this pull request Dec 22, 2022
Merged
@michaelpj
Copy link
Collaborator

What's the status of this?

@michaelpj michaelpj mentioned this pull request Jan 10, 2023
Merged
@wz1000 wz1000 force-pushed the wip/9.4-eval-plugin branch from 32a6ae3 to 0becd19 Compare January 23, 2023 06:51
@wz1000
Copy link
Collaborator Author

wz1000 commented Jan 23, 2023

This is ready for review

@pepeiborra setupDynFlagsForGHCiLike doesn't do much as setting the optimisation level to 0 sets the same set of flags.

@wz1000 wz1000 force-pushed the wip/9.4-eval-plugin branch 2 times, most recently from 2627c13 to 4541fef Compare January 25, 2023 12:48
@pepeiborra
Copy link
Collaborator

The plugin was implemented by calling "load" which circumvents all of HLSs caching mechanisms for interface files and linkables. Instead we should work like the other typechecking functions which get all the stuff we need using HLS rules and setup the HscEnv with all the state in the right places.

Is there any way to check this in a test? Or a benchmark? Seems like an important property.

@wz1000
Copy link
Collaborator Author

wz1000 commented Feb 15, 2023

The plugin was implemented by calling "load" which circumvents all of HLSs caching mechanisms for interface files and linkables. Instead we should work like the other typechecking functions which get all the stuff we need using HLS rules and setup the HscEnv with all the state in the right places.

Is there any way to check this in a test? Or a benchmark? Seems like an important property.

I don't thin there is an easy way to do this. We can blacklist the load family of functions using hlint.

@michaelpj
Copy link
Collaborator

Blacklisting the functions sounds good!

@wz1000 wz1000 force-pushed the wip/9.4-eval-plugin branch from 2762851 to 13f09c8 Compare February 16, 2023 10:52
@wz1000 wz1000 enabled auto-merge (rebase) February 16, 2023 10:52
@wz1000 wz1000 force-pushed the wip/9.4-eval-plugin branch from 13f09c8 to f17707c Compare February 16, 2023 10:54
@wz1000
Copy link
Collaborator Author

wz1000 commented Feb 16, 2023

I have added an hlint rule and also fixed a regression that caused the eval plugin not to work due to #3414 landing. See the comment in the last commit for details.

@wz1000 wz1000 force-pushed the wip/9.4-eval-plugin branch 3 times, most recently from ed2980f to 6a8d585 Compare February 21, 2023 13:18
@mpickering @wz1000
Simplify implementation of eval plugin and make it work with GHC 9.4
f012e04 
The plugin was implemented by calling "load" which circumvents all of
HLSs caching mechanisms for interface files and linkables. Instead we
should work like the other typechecking functions which get all the
stuff we need using HLS rules and setup the HscEnv with all the state in
the right places.

The key part to this is setting up all the HPT modules with linkables if
they are depenedencies of the module we are trying to run a function
from.

- ban load functions from GHC driver

- Enable CI for hls-eval-plugin and fix a bug due to clearing of mi_globals
@wz1000 wz1000 force-pushed the wip/9.4-eval-plugin branch from 6a8d585 to f012e04 Compare February 21, 2023 20:12
@wz1000 wz1000 merged commit f63627f into master Feb 22, 2023
@jhrcek jhrcek mentioned this pull request Feb 25, 2023
Closed
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

@Anton-Latukha Anton-Latukha Awaiting requested review from Anton-Latukha

@Ailrun Ailrun Awaiting requested review from Ailrun

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@wz1000 @michaelpj @pepeiborra @mpickering