-
Notifications
You must be signed in to change notification settings - Fork 81
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
refactor(data/{fin,unsigned}/*): remove unused {fin/unsigned} ops #527
refactor(data/{fin,unsigned}/*): remove unused {fin/unsigned} ops #527
Conversation
bors try |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Out of curiosity, do you happen to know where unsigned.add
is still used? I couldn't immediately find it by searching.
It's used in
Some parts of the code supply a numeral directly for However, I ripped out numeral support for |
Right, I saw that but I wasn't sure how the |
Given the future of Lean 4, I don't think this is the right direction. Lean 4 has |
The removed fin and unsigned ops are not used in core, other than
unsigned.add
. Moving them to mathlib would allow them to be redefined, if need be. Test outputs were edited to remove the now-absent instances.Zulip thread: https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/fin.20ops.20and.20unsigned.20ops.20in.20core
As far as I can tell, these functions have no VM overrides, so it is fine to remove these from core. This is a reattempt of #525, done with checking the build locally first.