-
-
Notifications
You must be signed in to change notification settings - Fork 364
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.
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
hls-eval-plugin: Replicate #4139 #4140
Conversation
Thanks! As a note, our CI jobs are rather flaky and fail unpredictably, so don't be afraid and suspect an error in your code if some random plugin tests fail. |
That's a good start! Perhaps now we can investigate why the other test cases are fine, and only this scenario is affected? Since it might not be obvious, you can run your individual test case locally like this: > TASTY_PATTERN="T4139" cabal test test:hls-eval-plugin-tests To see debug logs: > HLS_TEST_PLUGIN_LOG_STDERR=1 TASTY_PATTERN="T4139" cabal test test:hls-eval-plugin-tests And to log everything in HLS (likely too much information): > HLS_TEST_LOG_STDERR=1 TASTY_PATTERN="T4139" cabal test test:hls-eval-plugin-tests |
The debug output was not much of a help. In continuing the search, I tried explicitly filtering out the @fendor do you know if there are any specific properties of the module, or the hsc environment that it might be worth to check for differences? |
Unfortunately no, I am not familiar with that part of GHC's codebase. Perhaps @wz1000 knows. |
Continuing the search, I found the tcRnDeclsi in hscParsedDecls to be the point where the
|
Sounds like good detective work! You can share whatever you find! I will likely only get to it some time in the next couple of days. |
Attaching the diffs. They're produced by running |
Scratch the previous comment. It turned out to be a dead-end. After comparing with renamer traces with GHC It appears that for output-dump-9.4.8.txt |
Can we merge the test as an expected failure for now? It's a good contribution regardless! |
It might worth adding a eval benchmark test to see if switch to interpreter back end would cause any leak or undesired behaviour. You can add a experiment here |
@soulomoon I added a couple of experiments, one with very simple, single line evaluation, and one a bit more complex. After inspecting the output logs locally, the benchmarks seem to actually exercise the desired behavior. Is this what you had in mind though? |
Thank you for add new benchmarks, it is looking good. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.