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: discrepancy in the elaborators for theorem, def, and example #4482

Merged
merged 2 commits into from
Jun 27, 2024

Conversation

leodemoura
Copy link
Member

When the type of a definition or example is a proposition,
we should elaborate on them as we elaborate on theorems.
This is particularly important for examples that are often
used in educational material.

Recall that when elaborating theorem headers, we convert unassigned
universe metavariables into universe parameters. The motivation is
that the proof of a theorem should not influence its statement.
However, before this commit, this was not the case for definitions and
examples when their type was a proposition. This discrepancy often confused users.

Additionally, we considered extending the above behavior whenever
the type of a definition is provided. That is, we would keep the
current behavior only if : <type> was omitted in a definition.
However, this proved to be too restrictive.
For example, the following instance in Core.lean would fail:

instance {α : Sort u} [Setoid α] : HasEquiv α :=
  ⟨Setoid.r⟩

and we would have to write instead:

instance {α : Sort u} [Setoid α] : HasEquiv.{u, 0} α :=
  ⟨Setoid.r⟩

There are other failures like this in the core, and we assume many more in Mathlib.

closes #4398

@semorrison @jcommelin: what do you think?

@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc June 17, 2024 22:52 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 17, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jun 17, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jun 17, 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 17, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

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

Mathlib CI status (docs):

  • 💥 Mathlib branch lean-pr-testing-4482 build failed against this PR. (2024-06-17 23:06:47) View Log
  • 💥 Mathlib branch lean-pr-testing-4482 build failed against this PR. (2024-06-17 23:58:00) View Log
  • 💥 Mathlib branch lean-pr-testing-4482 build failed against this PR. (2024-06-24 01:24:55) View Log
  • 💥 Mathlib branch lean-pr-testing-4482 build failed against this PR. (2024-06-24 02:29:30) View Log
  • 💥 Mathlib branch lean-pr-testing-4482 build failed against this PR. (2024-06-24 02:51:39) View Log
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 18c97926a13bfc82c9266bf00483ab13c6acb64f --onto 49249b91074a8eab8926308cedc2b3e31d973b51. (2024-06-27 01:03:22)

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jun 17, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jun 17, 2024
@kim-em
Copy link
Collaborator

kim-em commented Jun 24, 2024

This would be great. I'll take a look at Mathlib.

There are problems in Mathlib, but they are mostly things that deserve to be fixed. Unfortunately the error messages are terrible, and it's completely unclear that the proofs are being rejected because they specialize the universe variables in the statement.

github-merge-queue bot pushed a commit that referenced this pull request Jun 24, 2024
When the type of an `example` is a proposition,
we should elaborate on them as we elaborate on theorems.
This is particularly important for examples that are often
used in educational material.

Recall that when elaborating theorem headers, we convert unassigned
universe metavariables into universe parameters. The motivation is
that the proof of a theorem should not influence its statement.
However, before this commit, this was not the case for examples when
their type was a proposition.
This discrepancy often confused users.

Additionally, we considered extending the above behavior to definitions
when
1- When their type is a proposition. However, it still caused disruption
in Mathlib.
2- When their type is provided. That is, we would keep the current
behavior only if `: <type>` was omitted. This would make the elaborator
for `def` much closer to the one for `theorem`, but it proved to be too
restrictive.
For example, the following instance in `Core.lean` would fail:
```
instance {α : Sort u} [Setoid α] : HasEquiv α :=
  ⟨Setoid.r⟩
```
and we would have to write instead:
```
instance {α : Sort u} [Setoid α] : HasEquiv.{u, 0} α :=
  ⟨Setoid.r⟩
```
There are other failures like this in the core, and we assume many more
in Mathlib.

closes #4398
closes #4482 Remark: PR #4482 implements option 1 above. We may consider
it again in the future.
@kim-em kim-em closed this in #4493 Jun 24, 2024
@kim-em kim-em reopened this Jun 24, 2024
@kim-em
Copy link
Collaborator

kim-em commented Jun 24, 2024

A typical example of an unhelpful error message is:

example (X : TopCat) (F : Presheaf CommRingCat X)
    (h : Presheaf.IsSheaf (F ⋙ (forget CommRingCat))) :
    F.IsSheaf :=
(isSheaf_iff_isSheaf_comp (forget CommRingCat) F).mpr h

which fails with

type mismatch
  (isSheaf_iff_isSheaf_comp (forget CommRingCat) ?m.3480).mpr ?m.3884
has type
  ?m.3480.IsSheaf : Prop
but is expected to have type
  F.IsSheaf : Prop

giving no clue that the fix is:

example (X : TopCat.{u₁}) (F : Presheaf CommRingCat.{u₁} X)
    (h : Presheaf.IsSheaf (F ⋙ (forget CommRingCat))) :
    F.IsSheaf :=
(isSheaf_iff_isSheaf_comp (forget CommRingCat) F).mpr h

@kim-em
Copy link
Collaborator

kim-em commented Jun 24, 2024

It turns out that that class of inscrutable error was actually very rare in Mathlib.

Instead I found many instances where authors had implicitly constrained universes via the RHS of the := in ways that were confusing and impossible to see from the type signature. I'm going to backport all the Mathlib fixes to master regardless of whether/when we merge this, but +1 from me!

mathlib-bors bot pushed a commit to leanprover-community/mathlib4 that referenced this pull request Jun 26, 2024
…ture. (#14069)

In many places in Mathlib universes in the type signature are invisibly constrained by the RHS of the `def`.

This PR makes all these explicit in the type signature.

Likely we will change the Lean behaviour to disallow this in leanprover/lean4#4482 (i.e. this is the backport to `master` of the fixes required for that).
kbuzzard pushed a commit to leanprover-community/mathlib4 that referenced this pull request Jun 26, 2024
…ture. (#14069)

In many places in Mathlib universes in the type signature are invisibly constrained by the RHS of the `def`.

This PR makes all these explicit in the type signature.

Likely we will change the Lean behaviour to disallow this in leanprover/lean4#4482 (i.e. this is the backport to `master` of the fixes required for that).
When the type of a definition or example is a proposition,
we should elaborate on them as we elaborate on theorems.
This is particularly important for examples that are often
used in educational material.

Recall that when elaborating theorem headers, we convert unassigned
universe metavariables into universe parameters. The motivation is
that the proof of a theorem should not influence its statement.
However, before this commit, this was not the case for definitions and
examples when their type was a proposition. This discrepancy often confused users.

Additionally, we considered extending the above behavior whenever
the type of a definition is provided. That is, we would keep the
current behavior only if `: <type>` was omitted in a definition.
However, this proved to be too restrictive.
For example, the following instance in `Core.lean` would fail:
```
instance {α : Sort u} [Setoid α] : HasEquiv α :=
  ⟨Setoid.r⟩
```
and we would have to write instead:
```
instance {α : Sort u} [Setoid α] : HasEquiv.{u, 0} α :=
  ⟨Setoid.r⟩
```
There are other failures like this in the core, and we assume many more in Mathlib.

closes #4398
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc June 27, 2024 00:21 Inactive
@leodemoura leodemoura added this pull request to the merge queue Jun 27, 2024
Merged via the queue into master with commit ee42c3c Jun 27, 2024
22 checks passed
dagurtomas pushed a commit to leanprover-community/mathlib4 that referenced this pull request Jul 2, 2024
…ture. (#14069)

In many places in Mathlib universes in the type signature are invisibly constrained by the RHS of the `def`.

This PR makes all these explicit in the type signature.

Likely we will change the Lean behaviour to disallow this in leanprover/lean4#4482 (i.e. this is the backport to `master` of the fixes required for that).
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.

simp doesn't rewrite because of unassigned metavariables
3 participants