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

fix: structural recursion: do not check for brecOn too early #4831

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

nomeata
Copy link
Collaborator

@nomeata nomeata commented Jul 25, 2024

Due to nested recursion, we do two passes of getRecArgInfo: One on
each argument in isolation, to see which inductive types are around
(e.g. Tree and List), and
then we later refine/replace this result with the data for the nested
type former (the implicit ListTree).

If we have nested recursion through a non-recursive data type like
Array or Prod then arguemnts of these types should survive the first
phase, so that we can still use them when looking for, say, Array Tree.

This was helpfully reported by @arthur-adjedj.

Due to nested recursion, we do two passes of `getRecArgInfo`: One on
each argument in isolation, to see which inductive types are around
(e.g. `Tree` and `List`), and
then we later refine/replace this result with the data for the nested
type former (the implicit `ListTree`).

If we have nested recursion through a non-recursive data type like
`Array` or `Prod` then arguemnts of these types should survive the first
phase, so that we can still use them when looking for, say, `Array
Tree`.

This was helpfully reported by @arthur-adjedj.
@nomeata nomeata added the will-merge-soon …unless someone speaks up label Jul 25, 2024
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc July 25, 2024 15:17 Inactive
@nomeata
Copy link
Collaborator Author

nomeata commented Jul 25, 2024

I think there is a way to not need an eventually unusable RecArgInfo in the first pass, with a little code restructuring. I’ll do that if the current code causes more problems in the future.

@nomeata nomeata added this pull request to the merge queue Jul 25, 2024
@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 Jul 25, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

Mathlib CI status (docs):

  • ❗ Batteries CI can not be attempted yet, as the nightly-testing-2024-07-25 tag does not exist there yet. We will retry when you push more commits. If you rebase your branch onto nightly-with-mathlib, Batteries CI should run now. (2024-07-25 15:26:14)

Merged via the queue into master with commit 54c22ef Jul 25, 2024
15 checks passed
@nomeata nomeata deleted the joachim/mutual-structural-array branch July 25, 2024 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

2 participants