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

fix: universe parameter order discrepancy between theorem and def #4408

Merged
merged 2 commits into from
Jun 11, 2024

Conversation

leodemoura
Copy link
Member

Before this commit, the theorem and def declarations had different
universe parameter orders.
For example, the following theorem:

theorem f (a : α) (f : α → β) : f a = f a := by
  rfl

was elaborated as

theorem f.{u_2, u_1} : ∀ {α : Sort u_1} {β : Sort u_2} (a : α) (f : α → β), f a = f a :=
  fun {α} {β} a f => Eq.refl (f a)

However, if we declare f as a def, the expected order is produced.

def f.{u_1, u_2} : ∀ {α : Sort u_1} {β : Sort u_2} (a : α) (f : α → β), f a = f a :=
  fun {α} {β} a f => Eq.refl (f a)

This commit fixes this discrepancy.

@semorrison @jcommelin: This might be a disruptive change to Mathlib, but it is better to fix the issue asap. I am surprised nobody has complained about this issue before. I discovered it while trying to reduce discrepancies between theorem and def elaboration.

Before this commit, the `theorem` and `def` declarations had different
universe parameter orders.
For example, the following `theorem`:
```
theorem f (a : α) (f : α → β) : f a = f a := by
  rfl
```
was elaborated as
```
theorem f.{u_2, u_1} : ∀ {α : Sort u_1} {β : Sort u_2} (a : α) (f : α → β), f a = f a :=
  fun {α} {β} a f => Eq.refl (f a)
```
However, if we declare `f` as a `def`, the expected order is produced.
```
def f.{u_1, u_2} : ∀ {α : Sort u_1} {β : Sort u_2} (a : α) (f : α → β), f a = f a :=
  fun {α} {β} a f => Eq.refl (f a)
```

This commit fixes this discrepancy.

@semorrison: This might be a disruptive change to Mathlib, but it is better to fix the issue asap. I am surprised nobody has complained about this issue before. I discovered it while trying to reduce discrepancies between `theorem` and `def`.
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc June 9, 2024 00:48 Inactive
@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Jun 9, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jun 9, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jun 9, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan release-ci Enable all CI checks for a PR, like is done for releases labels Jun 9, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented Jun 9, 2024

Mathlib CI status (docs):

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jun 9, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jun 9, 2024
@kim-em kim-em added this pull request to the merge queue Jun 10, 2024
Merged via the queue into master with commit a1c8a94 Jun 11, 2024
26 checks passed
mathlib-bors bot pushed a commit to leanprover-community/mathlib4 that referenced this pull request Jun 11, 2024
kim-em added a commit to leanprover-community/mathlib4 that referenced this pull request Jun 16, 2024
This includes adaptations for:

* leanprover/lean4#4430
* leanprover/lean4#4421
* leanprover/lean4#4357
* leanprover/lean4#4385
* leanprover/lean4#4408

Sorry that it's a few days worth in one go, for various reasons it was
hard to get to green during this period.

---------

Co-authored-by: Johan Commelin <johan@commelin.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan release-ci Enable all CI checks for a PR, like is done for releases toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants