Skip to content

[Merged by Bors] - chore: avoid lean3 style have/suffices#6964

Closed
kim-em wants to merge 3 commits intomasterfrom
stream_of_tactics
Closed

[Merged by Bors] - chore: avoid lean3 style have/suffices#6964
kim-em wants to merge 3 commits intomasterfrom
stream_of_tactics

Conversation

@kim-em
Copy link
Copy Markdown
Contributor

@kim-em kim-em commented Sep 5, 2023

Many proofs use the "stream of consciousness" style from Lean 3, rather than have ... := or suffices ... from/by.

This PR updates a fraction of these to the preferred Lean 4 style.

I think a good goal would be to delete the "deferred" versions of have, suffices, and let at the bottom of Mathlib.Tactic.Have

(Anyone who would like to contribute more cleanup is welcome to push directly to this branch.)


Open in Gitpod

@kim-em kim-em added awaiting-review awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. labels Sep 5, 2023
@ericrbg
Copy link
Copy Markdown
Contributor

ericrbg commented Sep 5, 2023

I personally think the style with have/suffices ... \. proof is really quite nice and it's a shame we're getting rid of it. The new from looks weird to me without the preceding comma, too.

@github-actions github-actions bot removed the awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. label Sep 5, 2023
@jcommelin
Copy link
Copy Markdown
Member

I think both styles are fine. @semorrison what do you see as the downside of the

have : bla
· proof

style?

@kim-em
Copy link
Copy Markdown
Contributor Author

kim-em commented Sep 6, 2023

I think this is a lean3-ism, which unnecessarily introduces an intermediate tactic state with multiple goals (immediately before the ·) which doesn't help either human or machine readers of the code.

This PR originated because I made a PR upstreaming the tactic supporting this style to Std, and @digama0 (correctly, I think!) told me this was a bad idea.

Supporting this syntax also makes parsing slightly more complicated, because Lean has to decide whether the core syntax or the overriden syntax from Mathlib is relevant. Of course it can cope, but why not make it unambiguous?

I think everything is better about the new style, and we should consciously move on from lean3-isms where possible, rather than holding on to them out of familiarity.

@digama0
Copy link
Copy Markdown
Member

digama0 commented Sep 6, 2023

One reason not to do this manually is that this syntactic transformation is within the realm of things we could do automatically (once we get full library syntax rewrites working).

@kim-em
Copy link
Copy Markdown
Contributor Author

kim-em commented Sep 6, 2023

Sorry, why is that a reason to not do it? Are you asking that this be left as a warm-up / practice problem for full library rewrites?

@digama0
Copy link
Copy Markdown
Member

digama0 commented Sep 7, 2023

It's fine if you want to do it now, but it might be a waste of work if a computer will soon automate the job.

@kim-em
Copy link
Copy Markdown
Contributor Author

kim-em commented Sep 7, 2023

I wasn't proposing to do anymore than are already here.

@sgouezel
Copy link
Copy Markdown
Contributor

sgouezel commented Sep 7, 2023

I have always difficulties with the syntax for suffices. Why isn't it suffices H : ... := by ... just like everywhere else?

Copy link
Copy Markdown
Contributor

@sgouezel sgouezel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's clearly much better like that, thanks!
bors d+

@bors
Copy link
Copy Markdown

bors bot commented Sep 7, 2023

✌️ semorrison can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@ghost ghost added delegated This pull request has been delegated to the PR author (or occasionally another non-maintainer). and removed awaiting-review labels Sep 7, 2023
Co-authored-by: sgouezel <sebastien.gouezel@univ-rennes1.fr>
@kim-em
Copy link
Copy Markdown
Contributor Author

kim-em commented Sep 7, 2023

bors merge

@github-actions github-actions bot added the ready-to-merge This PR has been sent to bors. label Sep 7, 2023
bors bot pushed a commit that referenced this pull request Sep 7, 2023
Many proofs use the "stream of consciousness" style from Lean 3, rather than `have ... := ` or `suffices ... from/by`.

This PR updates a fraction of these to the preferred Lean 4 style.

I think a good goal would be to delete the "deferred" versions of `have`, `suffices`, and `let` at the bottom of `Mathlib.Tactic.Have`

(Anyone who would like to contribute more cleanup is welcome to push directly to this branch.)



Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
@bors
Copy link
Copy Markdown

bors bot commented Sep 7, 2023

Pull request successfully merged into master.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title chore: avoid lean3 style have/suffices [Merged by Bors] - chore: avoid lean3 style have/suffices Sep 7, 2023
@bors bors bot closed this Sep 7, 2023
@bors bors bot deleted the stream_of_tactics branch September 7, 2023 23:49
kodyvajjha pushed a commit that referenced this pull request Sep 22, 2023
Many proofs use the "stream of consciousness" style from Lean 3, rather than `have ... := ` or `suffices ... from/by`.

This PR updates a fraction of these to the preferred Lean 4 style.

I think a good goal would be to delete the "deferred" versions of `have`, `suffices`, and `let` at the bottom of `Mathlib.Tactic.Have`

(Anyone who would like to contribute more cleanup is welcome to push directly to this branch.)



Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
mathlib-bors bot pushed a commit that referenced this pull request Feb 18, 2024
…suffices` (#10640)

No changes to tactic file, it's just boring fixes throughout the library.

This follows on from #6964.



Co-authored-by: sgouezel <sebastien.gouezel@univ-rennes1.fr>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
riccardobrasca pushed a commit that referenced this pull request Feb 18, 2024
…suffices` (#10640)

No changes to tactic file, it's just boring fixes throughout the library.

This follows on from #6964.



Co-authored-by: sgouezel <sebastien.gouezel@univ-rennes1.fr>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
riccardobrasca pushed a commit that referenced this pull request Feb 18, 2024
…suffices` (#10640)

No changes to tactic file, it's just boring fixes throughout the library.

This follows on from #6964.



Co-authored-by: sgouezel <sebastien.gouezel@univ-rennes1.fr>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
thorimur pushed a commit that referenced this pull request Feb 24, 2024
…suffices` (#10640)

No changes to tactic file, it's just boring fixes throughout the library.

This follows on from #6964.



Co-authored-by: sgouezel <sebastien.gouezel@univ-rennes1.fr>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
thorimur pushed a commit that referenced this pull request Feb 26, 2024
…suffices` (#10640)

No changes to tactic file, it's just boring fixes throughout the library.

This follows on from #6964.



Co-authored-by: sgouezel <sebastien.gouezel@univ-rennes1.fr>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
dagurtomas pushed a commit that referenced this pull request Mar 22, 2024
…suffices` (#10640)

No changes to tactic file, it's just boring fixes throughout the library.

This follows on from #6964.



Co-authored-by: sgouezel <sebastien.gouezel@univ-rennes1.fr>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

delegated This pull request has been delegated to the PR author (or occasionally another non-maintainer). ready-to-merge This PR has been sent to bors.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants