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

RFC: Make simp only warn on unused list elements #4483

Open
andrewmw94 opened this issue Jun 17, 2024 · 2 comments
Open

RFC: Make simp only warn on unused list elements #4483

andrewmw94 opened this issue Jun 17, 2024 · 2 comments
Labels
P-medium We may work on this issue if we find the time RFC accepted RFC is waiting for a corresponding PR (external or internal) RFC Request for comments

Comments

@andrewmw94
Copy link

andrewmw94 commented Jun 17, 2024

Proposal

I'd like to make it so simp only [A B] warns if B is not used.

How does this feature improve the user experience?
When updating code, we sometimes see broken proofs that report an error at some line after simp only [A B] even though the root cause of the issue is that B is no longer being applied. Warning when this happens will make fixing proofs easier.

Which Lean users and projects benefit most from this feature/change?
This feature was requested by the Cedar team, but in the Zulip discussion other Lean users said they would appreciate it.

Will this change streamline code maintenance or simplify its structure?
I'm not familiar enough with the Lean code base to offer an opinion.

Community Feedback

Proposed in this Zulip thread.

Summary
People seemed receptive to the idea, with the caveat that it will be difficult to deal with something like split <;> simp [A, B] where B may only be needed in one branch.

Currently, Lean will flag an unused import even though the import can't be removed because it is listed but not used (i.e., it imports B in the example). So adding this linter would make removing unused imports easier.

People also agree we need some way to disable the linter (so opt-in or opt-out). This means the linter will have to go in core so leanOptions can globally enable / disable the linter.

Impact

I believe the impact would be minor. It's an added convenience, but one that several people have said they would appreciate.

@andrewmw94 andrewmw94 added the RFC Request for comments label Jun 17, 2024
@eric-wieser
Copy link
Contributor

This means the linter will have to go in core so leanOptions can globally enable / disable the linter.

Or address #3403 first

@leanprover-bot leanprover-bot added the P-medium We may work on this issue if we find the time label Aug 2, 2024
@Kha Kha added the RFC accepted RFC is waiting for a corresponding PR (external or internal) label Aug 2, 2024
@Kha
Copy link
Member

Kha commented Aug 2, 2024

RFC accepted; likely this should apply to explicit arguments of simp as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-medium We may work on this issue if we find the time RFC accepted RFC is waiting for a corresponding PR (external or internal) RFC Request for comments
Projects
None yet
Development

No branches or pull requests

4 participants