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: termination_by structural #4542

Merged
merged 26 commits into from
Jul 1, 2024
Merged

Conversation

nomeata
Copy link
Contributor

@nomeata nomeata commented Jun 23, 2024

This implements the termination_by structural syntax proposed in #3909.

I went with termination_by structural over, say,
termination_by (config := {method := .structural}) mainly because it was
easier to get going (otherwise I’d have to look into how to define recursive
parsers, as Parser.config depends on term and termination_by is part of
term. But also because I find it more ergonomic and aesthetic as a user.
But syntax can still change.

The termination_by? syntax will no longer force well-founded recursion,
and instead the inferred termination_by structurally annotation will be shown
if structural termination is possible.

While I was it, this fixes #4546 the easy way (log errors about but otherwise
ignore incomplete termination_by sets for mutual recursion). Maybe we get
multiple replacements (#4551), but even then this this good behavior.

Involves a bit of shuffling around TerimationHints (now validated for a
clique already by PreDefinition.main) and TerminationArguments (now lifted
out of the WF namespace, and a bit simplified).

Fixes #3909

@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 Jun 23, 2024
@nomeata
Copy link
Contributor Author

nomeata commented Jun 23, 2024

More on my preferrence of termination_by structurally over termination_by (config := {method := .structural}):

As a user, these “record terms syntax as part of tactic syntax” feels very clumsy and heavy (not helped by the rather verbose (config := and )). This gives me the impression that setting options is meant for advanced users, for corner cases, not for “idiomatic lean”.

I get this impression for example from the fact the simp syntax offers simp only rather than simp (config := {onlyGivenTheorems := true), because simp only is a common, basic use case , while other simp options (memoize, failIfUnchanged) are clearly more advanced, and appear to be more aimed at users wrapping up the simp tactic within other user-facing tactic, or maybe even only intended for use from the MetaM level.

Maybe if the concrete syntax was a bit less heavy (no config :=, no need for := true for boolean flags) I’d come to a different conclusion.

@nomeata nomeata force-pushed the joachim/termination_by_structurally branch from 6e080f6 to 809aca2 Compare June 23, 2024 15:58
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented Jun 23, 2024

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 378b02921d930ff805b8111a1ece04abc905b963 --onto 24d51b90cc4adbc68386671bba4587d6e47188d6. (2024-06-23 16:10:10)
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 378b02921d930ff805b8111a1ece04abc905b963 --onto e3578c2f36c2d4fa9cc55584a7671c0c81c70ed9. (2024-06-24 12:07:44)
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 18c97926a13bfc82c9266bf00483ab13c6acb64f --onto 49249b91074a8eab8926308cedc2b3e31d973b51. (2024-06-26 12:47:09)
  • ✅ Mathlib branch lean-pr-testing-4542 has successfully built against this PR. (2024-06-28 19:55:27) View Log
  • ✅ Mathlib branch lean-pr-testing-4542 has successfully built against this PR. (2024-06-29 12:24:44) View Log
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 7f00767b1e123f550d06a0e740bfd4e663b5c6bf --onto 4d0b7cf66c140dd83f8c8634293047335a385026. (2024-07-01 16:15:00)

this is  in preparation for #4542, and extracts from `findRecArg` the
functionality for trying one particular argument.

It also refactors the code a bit. In particular

 * It reports errors in the order of the parameters, not the order of
   in which they are tried (it tries non-indices first).
 * For every argument it will say why it wasn't tried, even if the
   reason is quite obviously (fixed prefix, or `Prop`-typed etc.)

Therefore there is some error message churn.
@nomeata nomeata marked this pull request as ready for review June 24, 2024 11:58
@nomeata nomeata requested a review from Kha as a code owner June 24, 2024 11:58
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc June 24, 2024 12:04 Inactive
github-merge-queue bot pushed a commit that referenced this pull request Jun 26, 2024
this is  in preparation for #4542, and extracts from `findRecArg` the
functionality for trying one particular argument.

It also refactors the code a bit. In particular

 * It reports errors in the order of the parameters, not the order of
   in which they are tried (it tries non-indices first).
 * For every argument it will say why it wasn't tried, even if the
   reason is quite obviously (fixed prefix, or `Prop`-typed etc.)

Therefore there is some error message churn.
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc June 26, 2024 12:41 Inactive
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc June 26, 2024 13:29 Inactive
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc June 26, 2024 14:42 Inactive
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc June 27, 2024 21:48 Inactive
@nomeata nomeata closed this Jun 28, 2024
@nomeata nomeata reopened this Jun 28, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jun 28, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jun 28, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label Jun 28, 2024
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc June 29, 2024 11:21 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jun 29, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jun 29, 2024
@nomeata nomeata added this pull request to the merge queue Jul 1, 2024
@nomeata nomeata removed the awaiting-review Waiting for someone to review the PR label Jul 1, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 1, 2024
This reverts commit 686c2e4.
@nomeata nomeata enabled auto-merge July 1, 2024 15:48
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc July 1, 2024 16:09 Inactive
@nomeata nomeata added this pull request to the merge queue Jul 1, 2024
@nomeata nomeata removed this pull request from the merge queue due to a manual request Jul 1, 2024
@nomeata nomeata changed the title feat: termination_by structurally feat: termination_by structural Jul 1, 2024
@nomeata nomeata enabled auto-merge July 1, 2024 16:41
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc July 1, 2024 16:49 Inactive
@nomeata nomeata added this pull request to the merge queue Jul 1, 2024
Merged via the queue into master with commit fb0c46a Jul 1, 2024
13 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

termination_by? code action unergonomic for mutual recursion RFC: termination_by structurally x
3 participants