hint-0.9.0.7 improvements #160
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Warn right away about the common mistake of importing a module from the current package. Warn about the bug with implicit kinds. Document where hint looks for packages. Clarify the behaviour of 'loadModules'. Advertize that it is possible, but difficult, to run hint without ghc being installed.
as it was written, the test could have passed because 1 divided by 0 returned NaN or something, not because it threw an exception which was caught.
A very common use case for hint is to interpret code which has access to the same packages as the compiled program which calls hint. the easiest way to accomplish that is to compile that program using "cabal build --write-ghc-environment-files=always", as that will write down all the necessary information to a file which hint can read at runtime. This test makes sure that this common workflow continues to work in the future.
such as the fact that hint is implemented via the ghc library
first show the user whether the library looks like it might possibly fit their use case, and only then write the small print explaining the circumstances in which it might not.
Previously, we were adding the phantom module to the list of targets, and then loading all the targets, and then if there are more targets to load, we were setting the targets to those plus the phantom module, and then loading all the targets again. Now, we simply set the targets to those plus the phantom module, and then we load all the targets, once.
previously we were avoiding an extra LoadAllTargets when we knew that there were no modules to load, but now that there is only one call to LoadAllTargets, there is no reason to distinguish that case.
In the previous two commits, I said that installSupportModule was calling LoadAllTargets twice in a row. That was only true on ghc-9.4+; on older versions of ghc, the first call was actually a call to "LoadUpTo phantomModule". Now that there is only one call, "LoadUpTo phantomModule" is insufficient, because we want to load all the targets, not just the phantom module.
now that it always returns GHC.Succeeded, it might as well just return unit.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I want to make a release with #152 and #158, but I would like to address a few low-hanging fruits first:
And if I have time (probably not):
setImportsworkflowreinstallSupportModuleto only load the targets once