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] - chore(*): bump to lean 3.47.0 #16252

Closed
wants to merge 14 commits into from
Closed

Conversation

digama0
Copy link
Member

@digama0 digama0 commented Aug 25, 2022

A major change is that notations now require names when they are shadowing another identical notation, even if it is a local notation. Also, because localized notations can be imported in a variety of contexts, there are some new best practices for them:

  • localized notations should always have a (name := ...). Notation names are unrelated to declaration names, but the declaration name is a reasonable base for the notation name.
  • localized notations should never use _ in the notation, because this gets desugared to a unique metavariable index, meaning that the notation will not be recognized as a duplicate of itself if open_locale is used when the notation is already available. Instead, you should use the hole! notation, which unfolds to _.

Another major change is that projection notation (x.foo) now always instantiates implicit arguments with metavariables, which is consistent with Lean 4. To simulate the older behavior, one can use either strict implicit arguments for the structure field (e.g. ∀ {{n}}, p n instead of ∀ {n}, p n) or, depending on specifics, writing λ _, x.foo to ensure the implicit argument is preserved as an argument.

@digama0 digama0 added the WIP Work in progress label Aug 25, 2022
@bryangingechen

This comment was marked as resolved.

@kmill
Copy link
Collaborator

kmill commented Aug 26, 2022

I notice you're going through a cycle of fixing issues with projection notation. I just want to be sure you know that the changes I submitted worked, so if you're switching to strict implicit arguments for stylistic reasons then feel free to carry on.

I also added a blurb to the PR description about the breaking change to projection notation.

@digama0
Copy link
Member Author

digama0 commented Aug 26, 2022

Yes, unfortunately what happened is that your notification that you had pushed the changes happened exactly when I had finished my own local compile of mathlib, including the projection fixes. So the cycle of fixes now is only the result of merge conflicts between our two (individually working) changes.

@kmill
Copy link
Collaborator

kmill commented Aug 26, 2022

I see -- if I had known you were also doing projection fixes I'd have left it to you! When you said on Zulip that it was good to know I was working on it, I was under the impression that this meant that you weren't. Now we're spending more than twice the time on this collectively than necessary.

@digama0 digama0 added awaiting-review The author would like community review of the PR and removed WIP Work in progress labels Aug 30, 2022
@gebner
Copy link
Member

gebner commented Aug 30, 2022

bors r+

@github-actions github-actions bot added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed awaiting-review The author would like community review of the PR labels Aug 30, 2022
bors bot pushed a commit that referenced this pull request Aug 30, 2022
A major change is that notations now require names when they are shadowing another identical notation, even if it is a local notation. Also, because localized notations can be imported in a variety of contexts, there are some new best practices for them:

* localized notations should always have a `(name := ...)`. Notation names are unrelated to declaration names, but the declaration name is a reasonable base for the notation name.
* localized notations should never use `_` in the notation, because this gets desugared to a unique metavariable index, meaning that the notation will not be recognized as a duplicate of itself if `open_locale` is used when the notation is already available. Instead, you should use the `hole!` notation, which unfolds to `_`.

Another major change is that projection notation (`x.foo`) now always instantiates implicit arguments with metavariables, which is consistent with Lean 4. To simulate the older behavior, one can use either strict implicit arguments for the structure field (e.g. `∀ {{n}}, p n` instead of `∀ {n}, p n`) or, depending on specifics, writing `λ _, x.foo` to ensure the implicit argument is preserved as an argument.

Co-authored-by: Kyle Miller <kmill31415@gmail.com>
@YaelDillies
Copy link
Collaborator

It's a bit sad seeing the heuristic that you can fill the op_mem field of a subobject_class instance with λ s, s.op_mem' breaking.

@bors
Copy link

bors bot commented Aug 30, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title chore(*): bump to lean 3.47.0 [Merged by Bors] - chore(*): bump to lean 3.47.0 Aug 30, 2022
@bors bors bot closed this Aug 30, 2022
@bors bors bot deleted the notation_name branch August 30, 2022 13:54
@kmill
Copy link
Collaborator

kmill commented Aug 30, 2022

@YaelDillies Maybe we should take this as motivation to use {{...}} arguments more often (though it would be nice to have some way to easily turn these into standard implicit arguments).

Consider also that projections now elaborate the same way as any other function (for example, it's id : ?M_1 → ?M_1 not id : Π {α : Sort u_1}, α → α).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants