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: a non-terminal simp linter #11246

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open

feat: a non-terminal simp linter #11246

wants to merge 21 commits into from

Conversation

adomani
Copy link
Collaborator

@adomani adomani commented Mar 8, 2024

An implementation of a syntax+InfoTrees linter checking for non-terminal simps.

The linter equates "non-terminal" with "does not strictly decrease the number of goals".

Accompanying PRs squeezing a few of the simps flagged by the linter:

Zulip threads:


Open in Gitpod

@adomani adomani added awaiting-review The author would like community review of the PR t-meta Tactics, attributes or user commands labels Mar 8, 2024
mathlib-bors bot pushed a commit that referenced this pull request Mar 8, 2024
This PR accompanies #11246, squeezing some non-terminal `simp`s highlighted by the linter until I decided to stop!
mathlib-bors bot pushed a commit that referenced this pull request Mar 10, 2024
The squeezing continues!  All found by the linter at #11246.
kbuzzard pushed a commit that referenced this pull request Mar 12, 2024
This PR accompanies #11246, squeezing some non-terminal `simp`s highlighted by the linter until I decided to stop!
kbuzzard pushed a commit that referenced this pull request Mar 12, 2024
The squeezing continues!  All found by the linter at #11246.
@alexjbest
Copy link
Member

This is great!

Is it possible to add a whitelist for some following tactics, for instance it has been agreed on zulip in the past that some patterns like simp; ring shouldn't really count as nonterminal simps as if ring solved the goal before then even if the simp normal form changes a bit ring should still close it (I'm not actually sure if this is 100% true, but it feels like simp; ring is indeed a maintainable pattern)

dagurtomas pushed a commit that referenced this pull request Mar 22, 2024
This PR accompanies #11246, squeezing some non-terminal `simp`s highlighted by the linter until I decided to stop!
dagurtomas pushed a commit that referenced this pull request Mar 22, 2024
The squeezing continues!  All found by the linter at #11246.
utensil pushed a commit that referenced this pull request Mar 26, 2024
This PR accompanies #11246, squeezing some non-terminal `simp`s highlighted by the linter until I decided to stop!
utensil pushed a commit that referenced this pull request Mar 26, 2024
The squeezing continues!  All found by the linter at #11246.
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the merge-conflict The PR has a merge conflict with master, and needs manual merging. label Apr 7, 2024
Louddy pushed a commit that referenced this pull request Apr 15, 2024
This PR accompanies #11246, squeezing some non-terminal `simp`s highlighted by the linter until I decided to stop!
Louddy pushed a commit that referenced this pull request Apr 15, 2024
The squeezing continues!  All found by the linter at #11246.
uniwuni pushed a commit that referenced this pull request Apr 19, 2024
This PR accompanies #11246, squeezing some non-terminal `simp`s highlighted by the linter until I decided to stop!
uniwuni pushed a commit that referenced this pull request Apr 19, 2024
The squeezing continues!  All found by the linter at #11246.
callesonne pushed a commit that referenced this pull request Apr 22, 2024
This PR accompanies #11246, squeezing some non-terminal `simp`s highlighted by the linter until I decided to stop!
callesonne pushed a commit that referenced this pull request Apr 22, 2024
The squeezing continues!  All found by the linter at #11246.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review The author would like community review of the PR merge-conflict The PR has a merge conflict with master, and needs manual merging. t-meta Tactics, attributes or user commands
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants