8000 Part of #2829: Allow instance arguments in pattern synonyms that are such in the pattern already by andreasabel · Pull Request #7173 · agda/agda · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Part of #2829: Allow instance arguments in pattern synonyms that are such in the pattern already #7173

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

Merged
merged 3 commits into from
Mar 8, 2024

Conversation

andreasabel
Copy link
Member
@andreasabel andreasabel commented Mar 7, 2024

3 commits, checked by CI individually:

1. Refactor for #2829: Add MetaKind(InstanceMeta,UnificationMeta) to MetaInfo

We can now remember in the abstract syntax Underscore which kind of meta we want to generate: a UnificationMeta (default) or an InstanceMeta. The latter is not used yet here but will be needed for {{_}} arguments created by the expansion of pattern synonyms.

Having data MetaKind(InstanceMeta,UnificationMeta) we also refactor MetaInstantiation(Open,OpenInstance) into a single parameterized constructor OpenMeta MetaKind. This helps condensing pattern matching in several locations, where Open and OpenInstance behaved the same.
I considered keeping Open and OpenInstance as pattern synonym but it wasn't worth it.

There was already a type MetaKind for directing eta-expansion for metas. This is a more obscure use than specifying the solution strategy for a meta, so I bumped this, calling it MetaClass now. This type is local to a only couple of modules, so renaming seems ok.

Changing the order of arguments in Rules.Term.checkUnderscore and friends seemed also natural.

Since newInteractionMetaArg was never used to create an instance meta, I simplified its code.

2. Allow instance arguments in pattern synonyms lhss...

... if they are instance arguments on the rhs as well.
To check this, we equip the scope checker with Hiding information for PatternBound local variables, and propagate the hiding info from Arg to the pattern scope checker by means of a new throw-away type WithHidingInfo (to not clash with the existing WithHiding).

This commit achieves that instance arguments that come to surface through pattern matching on a pattern synoym are available for instance search, just if one would match against the pattern the synonym stands for.

3. Make instance arguments in pattern synonyms work in expressions.

If a pattern synonym is used in an expression, we need to make sure that its instance arguments become instance metas.

Agda only turns an instance argument into an instance meta when the argument is entirely omitted. That is, {{_}} does not create an instance meta, but this is exactly what the pattern synonym expansion produces in expressions.

Having added metaKind to the MetaInfo record, we can now create instance underscores in the abstract syntax, which fixes our dilemma.

@andreasabel andreasabel added pattern-synonyms pr: preserve commits PR should be merged via rebase, preserving the commits labels Mar 7, 2024
@andreasabel andreasabel self-assigned this Mar 7, 2024
@andreasabel andreasabel added this to the 2.7.0 milestone Mar 7, 2024
@andreasabel andreasabel changed the title Refactor for #2829: Add MetaKind(InstanceMeta,UnificationMeta) to MetaInfo Part of #2829: Allow instance arguments in pattern synonyms that are such in the pattern already Mar 7, 2024
@andreasabel andreasabel requested a review from UlfNorell March 7, 2024 21:14
@andreasabel andreasabel added type: enhancement Issues and pull requests about possible improvements instance Instance resolution labels Mar 7, 2024
@andreasabel andreasabel requested a review from jespercockx March 7, 2024 21:15
Copy link
Member
@UlfNorell UlfNorell left a comment

Choose a reason for hiding this comment

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

👍🏻

…aInfo

We can now remember in the abstract syntax `Underscore` which kind of
meta we want to generate: a `UnificationMeta` (default) or an
`InstanceMeta`.  The latter is not used yet here but will be needed
for `{{_}}` arguments created by the expansion of pattern synonyms.

Having `data MetaKind(InstanceMeta,UnificationMeta)` we also refactor
`MetaInstantiation(Open,OpenInstance)` into a single parameterized
constructor `OpenMeta MetaKind`.  This helps condensing pattern
matching in several locations, where `Open` and `OpenInstance` behaved
the same.
I considered keeping `Open` and `OpenInstance` as pattern synonym but
it wasn't worth it.

There was already a type `MetaKind` for directing eta-expansion for
metas.  This is a more obscure use than specifying the solution
strategy for a meta, so I bumped this, calling it `MetaClass` now.
This type is local to a only couple of modules, so renaming seems ok.

Changing the order of arguments in `Rules.Term.checkUnderscore` and
friends seemed also natural.

Since `newInteractionMetaArg` was never used to create an instance
meta, I simplified its code.
... if they are instance arguments on the rhs as well.
To check this, we equip the scope checker with `Hiding` information
for `PatternBound` local variables, and propagate the hiding info from
`Arg` to the pattern scope checker by means of a new throw-away type
`WithHidingInfo` (to not clash with the existing `WithHiding`).

This commit achieves that instance arguments that come to surface
through pattern matching on a pattern synoym are available for
instance search, just if one would match against the pattern the
synonym stands for.
…ions.

If a pattern synonym is used in an expression, we need to make sure
that its instance arguments become instance metas.

Agda only turns an instance argument into an instance meta when the
argument is entirely omitted.  That is, `{{_}}` does not create an
instance meta, but this is exactly what the pattern synonym expansion
produces in expressions.

Having added `metaKind` to the `MetaInfo` record, we can now create
instance underscores in the abstract syntax, which fixes our dilemma.
@andreasabel
Copy link
Member Author

Added changelog entry.

Copy link
Member
@jespercockx jespercockx left a comment

Choose a reason for hiding this comment

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

Great work, it's indeed much better to have a different abstract syntax for regular implicits and instance implicits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
instance Instance resolution pattern-synonyms pr: preserve commits PR should be merged via rebase, preserving the commits type: enhancement Issues and pull requests about possible improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0