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

feat: mutual recursion: allow common prefix up to alpha-equivalence #5041

Merged
merged 2 commits into from
Aug 19, 2024

Conversation

nomeata
Copy link
Collaborator

@nomeata nomeata commented Aug 14, 2024

@arthur-adjedj was very confused when a mutually recursive definition
didn't work as expected, and the reason was that he used different names
for the fixed parameters.

It seems plausible to simply allow that and calculate the fixed-prefix
up to alpha renaming.

It does mean, though, that, for example, termination proof goals will
mention the names as used by the first function. But probably better
than simply failing. And we could even fix that later (by passing down the
actual names, and renmaing the variables in the context of the mvar,
depending on the “current function”) should it bother our users.

@arthur-adjedj was very confused when a mutually recursive definition
didn't work as expected, and the reason was that he used different names
for the fixed parameters.

It seems plausible to simply allow that and calculate the fixed-prefix
up to alpha renaming.

It does mean, though, that, for example, termination proof goals will
mention the names as used by the first function. But probably better
than simply failing. And we could even fix that later (by passing down the
actual names, and renmaing the variables in the context of the mvar,
depending on the “current function”) should it bother our users.
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc August 14, 2024 12:59 Inactive
@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Aug 14, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Aug 14, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Aug 14, 2024
@arthur-adjedj
Copy link
Contributor

To give some details: I encountered the issue when looking at code generated by #3160. The way in which nested deriving is implemented there does not always lead to having the exact same variable names between each definition in a mutual block. I could work around this if needed, but I think it could be desirable for common-prefixes to be determined up to alpha-equivalence.

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label Aug 14, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

Mathlib CI status (docs):

@nomeata nomeata marked this pull request as ready for review August 14, 2024 14:36
@nomeata nomeata added awaiting-review Waiting for someone to review the PR will-merge-soon …unless someone speaks up and removed awaiting-review Waiting for someone to review the PR labels Aug 14, 2024
@nomeata
Copy link
Collaborator Author

nomeata commented Aug 19, 2024

@arthur-adjedj, this got green light.

@nomeata nomeata added this pull request to the merge queue Aug 19, 2024
Merged via the queue into master with commit 7814619 Aug 19, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builds-mathlib CI has verified that Mathlib builds against this PR toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN will-merge-soon …unless someone speaks up
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants