8000 feat: linter.loopingSimpArgs by nomeata · Pull Request #8865 · leanprover/lean4 · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat: linter.loopingSimpArgs #8865

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 84 commits into from
Jun 23, 2025
Merged

feat: linter.loopingSimpArgs #8865

merged 84 commits into from
Jun 23, 2025

Conversation

nomeata
Copy link
Collaborator
@nomeata nomeata commented Jun 18, 2025

This PR allows simp to recognize and warn about simp lemmas that are likely looping in the current simp set. It does so automatically whenever simplification fails with the dreaded “max recursion depth” error fails, but it can be made to do it always with set_option linter.loopingSimpArgs true. This check is not on by default because it is somewhat costly, and can warn about simp calls that still happen to work.

This closes #5111. In the end, this implemented much simpler logic than described there (and tried in the abandoned #8688; see that PR description for more background information), but it didn’t work as well as I thought. The current logic is:

“Simplify the RHS of the simp theorem, complain if that fails”.

It is a reasonable policy for a Lean project to say that all simp invocation should be so that this linter does not complain. Often it is just a matter of explicitly disabling some simp theorems from the default simp set, to make it clear and robust that in this call, we do not want them to trigger. But given that often such simp call happen to work, it’s too pedantic to impose it on everyone.

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jun 20, 2025
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jun 20, 2025
@nomeata nomeata changed the title feat: linter.simp.loopProtection feat: linter.simp.loopingSimpArgs Jun 22, 2025
@nomeata nomeata changed the title feat: linter.simp.loopingSimpArgs feat: linter.loopingSimpArgs Jun 22, 2025
@nomeata nomeata mentioned this pull request Jun 22, 2025
github-merge-queue bot pushed a commit that referenced this pull request Jun 22, 2025
This PR factors out the common code for running `SimpM` from `mainCore`
and `dsimpMainCore`, and make it available separately (e.g. for #8865).
@nomeata nomeata force-pushed the joachim/simp-loop-detection2 branch from ec22de4 to a17b098 Compare June 22, 2025 15:08
@nomeata nomeata marked this pull request as ready for review June 22, 2025 15:12
@nomeata nomeata added this pull request to the merge queue Jun 23, 2025
Merged via the queue into master with commit 29298c9 Jun 23, 2025
16 checks passed
wkrozowski pushed a commit to wkrozowski/lean4 that referenced this pull request Jun 24, 2025
This PR factors out the common code for running `SimpM` from `mainCore`
and `dsimpMainCore`, and make it available separately (e.g. for leanprover#8865).
wkrozowski pushed a commit to wkrozowski/lean4 that referenced this pull request Jun 24, 2025
This PR allows `simp` to recognize and warn about simp lemmas that are
likely looping in the current simp set. It does so automatically
whenever simplification fails with the dreaded “max recursion depth”
error fails, but it can be made to do it always with `set_option
linter.loopingSimpArgs true`. This check is not on by default because it
is somewhat costly, and can warn about simp calls that still happen to
work.

This closes leanprover#5111. In the end, this implemented much simpler logic than
described there (and tried in the abandoned leanprover#8688; see that PR
description for more background information), but it didn’t work as well
as I thought. The current logic is:

“Simplify the RHS of the simp theorem, complain if that fails”.

It is a reasonable policy for a Lean project to say that all simp
invocation should be so that this linter does not complain. Often it is
just a matter of explicitly disabling some simp theorems from the
default simp set, to make it clear and robust that in this call, we do
not want them to trigger. But given that often such simp call happen to
work, it’s too pedantic to impose it on everyone.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan changelog-language Language features, tactics, and metaprograms merge-ci Enable merge queue CI checks for PR. In particular, produce artifacts for all major platforms. 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.

RFC: simp to detect, report and ignore inherently looping rewrite rules
3 participants
0