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.

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

Initial version of typed holes support under allow-typed-holes flag #2486

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

matheussbernardo
Copy link

Based on PR#1140

I am adding a flag --allow-typed-holes
and detecting holes which are bindings with hole as infix.

These holes are currently being saved in the CGInfo and a warning is being presented to the user showing the refinement type.

I added one example in the test that shows how to use this experimental feature.

Copy link
Collaborator

@facundominguez facundominguez left a comment

Choose a reason for hiding this comment

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

Thanks @matheussbernardo. I left some comments. Can we have the feature documented? Maybe it could have a section in docs/mkDocs/docs/options.md.

Also, it would be helpful to have an example of a warning message produced by this implementation for the discussion.

getSrcSpanFromTick = case genTick of
SourceNote src _ -> src
_ -> panic Nothing "Not a Source Note"
isVarHole = L.isInfixOf "hole" . F.symbolString . F.symbol
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will this mistakenly catch variables written by the user that include "hole" in their name? e.g. whole, wholesale, etc.

To make this as precise as possible, I need to understand yet how the variable is bound.

Copy link
Author

Choose a reason for hiding this comment

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

I updated the implementation. Can you take a second look?

@@ -1,6 +1,6 @@
{-@ LIQUID "--expect-error-containing=Hole Found" @-}
{-@ LIQUID "--exact-data-cons" @-}
{-@ LIQUID "--allow-typed-holes" @-}
{-@ LIQUID "--warn-on-term-holes" @-}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This could be added to the basic-pos testsuite. For scripts/test/test_plugin.sh to pick up the module, it will need to be listed in tests/tests.cabal.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I just found that it is actually listed in tests.cabal. A separate executable seems overkill for a single test. It would be terser to put it in an existing executable.

Copy link
Author

Choose a reason for hiding this comment

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

I added more tests to this. I was thinking of adding all my examples to it. But maybe it is a overkill

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants