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

[Merged by Bors] - feat: notation3 delaborator generation #4533

Closed
wants to merge 12 commits into from

Conversation

kmill
Copy link
Contributor

@kmill kmill commented Jun 1, 2023

Gives the notation3 command the ability to generate a delaborator in most common situations. When it succeeds, notation3-defined syntax is pretty printable.

Examples:

(⋃ (i : ι) (i' : ι'), s i i') = ⋃ (i' : ι') (i : ι), s i i'
(⨆ (g : α → ℝ≥0∞) (_ : Measurable g) (_ : g ≤ f), ∫⁻ (a : α), g a ∂μ) = ∫⁻ (a : α), f a ∂μ

Open in Gitpod

@kmill kmill added the t-meta Tactics, attributes or user commands label Jun 1, 2023
@kmill kmill added the awaiting-review The author would like community review of the PR label Jun 1, 2023
@kmill
Copy link
Contributor Author

kmill commented Jun 1, 2023

@digama0 It's still in CI, but in my local testing it seems to work. It's an extension to the notation3 command to generate a delaborator, and the output is comparable to what we used to have in Lean 3.

I'm trying to add some usability improvement to mathlib in time for the Lean workshop at MSRI that starts next week, so it would be helpful having an expedited review, though worst case we could use a custom branch of mathlib for TPIL.

notation3"c["(l", "* => foldr (h t => List.cons h t) List.nil)"]" =>
Cycle.formPerm (l) (Cycle.nodup_coe_iff.mpr _)
notation3 "c["(l", "* => foldr (h t => List.cons h t) List.nil)"]" =>
Cycle.formPerm (Cycle.ofList l) (Iff.mpr Cycle.nodup_coe_iff _)
Copy link
Contributor Author

@kmill kmill Jun 1, 2023

Choose a reason for hiding this comment

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

Just to explain: the Syntax needs to get turned into an Expr pattern for use in the delaborator, and pattern construction needs to be able to walk along the syntax without elaborating it. The walker only handles just enough Syntax features to be useful, and that does not include or dot notation.

This is not necessary for the syntax to work, but this change causes notation3 to generate a delaborator. set_option trace.notation3 true gives some information about its successes and failures.

Copy link
Member

Choose a reason for hiding this comment

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

It looks like the syntax here could be reconstructed from the elaborated expression though, although maybe you can't even elaborate this expression in context?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd thought about trying to elaborate the expressions to generate patterns, but then I wasn't sure if there'd be other things that would introduce into the expressions that'd get in the way of being a pattern, like maybe the elaborator might fill in some instance arguments with instances it wouldn't choose for some concrete arguments. With this syntax walker, I felt more confidant that it's doing the right thing in the cases it accepts.

The code's set up where you can swap out mkExprMatcher for a fancier implementation

@digama0
Copy link
Member

digama0 commented Jun 1, 2023

What fraction of existing notation3's are delaborated successfully? I think it would be better to have an explicit indication of whether the delaborator is constructed or not, e.g. by a warning that users have to suppress by some additional syntax (like lean 3 @[parsing_only], which maybe we would spell (parsing_only := true) now).

@kmill
Copy link
Contributor Author

kmill commented Jun 1, 2023

In principle there should only be three that don't work right now. ∀ᵐ and ∃ᵐ use <| in a superficial way, and it's easily fixed either in the declarations or more generally by macro expanding syntax before generating patterns.

The notation for Lex in Mathlib.Data.Pi.Lex seems trickier to support, and maybe that one deserves (parsing_only := true).

I'll take a look at them all tomorrow more carefully once I have a compiled mathlib.

@kmill
Copy link
Contributor Author

kmill commented Jun 1, 2023

@digama0 I added a (prettyPrint := false) option to notation3. It's running through CI, but as far as I understand only one notation3 needed it, and everything else delaborates.

@kmill
Copy link
Contributor Author

kmill commented Jun 1, 2023

@digama0 It builds, and I did a spot check for each notation to see that they delaborate, especially the Lesbegue integral notation (no need for its unexpanders now).

@digama0
Copy link
Member

digama0 commented Jun 2, 2023

As I discussed with @kmill privately, I have a rewrite of large parts of this code to move more of the matcher algorithm to compile time. But this implementation sets bar for the intended behavior of the delaborator, and I have no complaints about the code itself. So let's get this in the hands of mathlib users.

bors r+

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review The author would like community review of the PR labels Jun 2, 2023
@ChrisHughes24
Copy link
Member

Conflicts

bors r-

@bors
Copy link

bors bot commented Jun 2, 2023

Canceled.

@ChrisHughes24 ChrisHughes24 added awaiting-author A reviewer has asked the author a question or requested changes and removed ready-to-merge This PR has been sent to bors. labels Jun 2, 2023
@semorrison semorrison added the merge-conflict The PR has a merge conflict with master, and needs manual merging. label Jun 2, 2023
@digama0
Copy link
Member

digama0 commented Jun 2, 2023

bors r+

@bors
Copy link

bors bot commented Jun 2, 2023

👎 Rejected by label

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-author A reviewer has asked the author a question or requested changes labels Jun 2, 2023
@digama0 digama0 removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. label Jun 2, 2023
@digama0
Copy link
Member

digama0 commented Jun 2, 2023

bors r+

bors bot pushed a commit that referenced this pull request Jun 2, 2023
Gives the `notation3` command the ability to generate a delaborator in most common situations. When it succeeds, `notation3`-defined syntax is pretty printable.

Examples:
```
(⋃ (i : ι) (i' : ι'), s i i') = ⋃ (i' : ι') (i : ι), s i i'
(⨆ (g : α → ℝ≥0∞) (_ : Measurable g) (_ : g ≤ f), ∫⁻ (a : α), g a ∂μ) = ∫⁻ (a : α), f a ∂μ
```



Co-authored-by: Mario Carneiro <di.gama@gmail.com>
@bors
Copy link

bors bot commented Jun 2, 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 feat: notation3 delaborator generation [Merged by Bors] - feat: notation3 delaborator generation Jun 2, 2023
@bors bors bot closed this Jun 2, 2023
@bors bors bot deleted the kmill_pp_notation3 branch June 2, 2023 13:42
qawbecrdtey pushed a commit to qawbecrdtey/greedoid-mathlib4 that referenced this pull request Jun 12, 2023
Gives the `notation3` command the ability to generate a delaborator in most common situations. When it succeeds, `notation3`-defined syntax is pretty printable.

Examples:
```
(⋃ (i : ι) (i' : ι'), s i i') = ⋃ (i' : ι') (i : ι), s i i'
(⨆ (g : α → ℝ≥0∞) (_ : Measurable g) (_ : g ≤ f), ∫⁻ (a : α), g a ∂μ) = ∫⁻ (a : α), f a ∂μ
```



Co-authored-by: Mario Carneiro <di.gama@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has been sent to bors. t-meta Tactics, attributes or user commands
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants