-
Notifications
You must be signed in to change notification settings - Fork 380
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 by Bors] - chore: robustifying for debug.byAsSorry #14993
Conversation
PR summary 77fd010eb8Import changes for modified filesNo significant changes to the import graph Import changes for all files
Declarations diff
You can run this locally as follows## summary with just the declaration names:
./scripts/declarations_diff.sh <optional_commit>
## more verbose report:
./scripts/declarations_diff.sh long <optional_commit> |
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.
Thanks for the clean-up! I agree that these changes are neutral or positive. I left a couple of small tweaks, but in my eyes, this PR could be delegated easily. Hence, let me
maintainer merge
|
||
/-- Transferring from `IsField` to `Field`. -/ | ||
noncomputable def IsField.toField {R : Type u} [Ring R] (h : IsField R) : Field R := | ||
{ ‹Ring R›, IsField.toSemifield h with qsmul := _ } | ||
{ ‹Ring R›, IsField.toSemifield h with |
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.
Just flagging for the other reviewers: I don't have an opinion on these - I trust you these are necessary later.
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.
This change is harmless, but I don't understand why it is needed.
Clearly the _
is being populated by elaborating the rfl
proof, but why is this impacted by whether the rfl
proof is inlined or inserted automatically behind the scenes?
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.
Under byAsSorry
, rfl
will never even run.
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.
Oh, is the difference here by rfl
vs rfl
?
🚀 Pull request has been placed on the maintainer queue by grunweg. |
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.
bors d+
✌️ semorrison can now approve this pull request. To approve and merge a pull request, simply reply with |
Can you explain in the PR description what |
Co-authored-by: grunweg <[email protected]>
bors merge |
These are some minor changes, which are think are positive or neutral in any case, that help Mathlib compile under `set_option debug.byAsSorry true` (an option that [arrived](leanprover/lean4#4576) in v4.10 that turns all `by` blocks into sorries
Pull request successfully merged into master. Build succeeded: |
Followup to #14993. Hopefully these are all independently positive or neutral. These help get Mathlib compiling under `set_option debug.byAsSorry true`.
These are some minor changes, which are think are positive or neutral in any case, that help Mathlib compile under
set_option debug.byAsSorry true
(an option that arrived in v4.10 that turns allby
blocks into sorries --- Mathlib is very much broken, although perhaps less than one might expect).