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: use maxType when building expression in expression tree elaborator #4215

Merged
merged 2 commits into from
May 18, 2024

Conversation

kmill
Copy link
Collaborator

@kmill kmill commented May 18, 2024

The expression tree elaborator computes a "maxType" that every leaf term can be coerced to, but the elaborator was not ensuring that the entire expression tree would have maxType as its type. This led to unexpected errors in examples such as

example (a : Nat) (b : Int) :
  a = id (a * b^2) := sorry

where it would say it could not synthesize an HMul Int Int Nat instance (the Nat would propagate from the a on the LHS of the equality). The issue in this case is that HPow uses default instances, so while the expression tree elaborator decides that a * b^2 should be referring to an Int, the actual elaborated type is temporarily a metavariable. Then, when the binrel elaborator is looking at both sides of the equality, it decides that Nat will work and coercions don't need to be inserted.

The fix is to unify the type of the resulting elaborated expression with the computed maxType. One wrinkle is that hasUncomparable being false is a valid test only if there are no leaf terms with unknown types (if they become known, it could change hasUncomparable to true), so this unification is only performed if the leaf terms all have known types.

Fixes issue described by Floris van Doorn on Zulip.

After the expression tree elaborator inserts coercions, it builds the expression itself. It was not storing the computed maxType in the return types of operators. This led to unexpected errors in examples such as
```lean
example (a : Nat) (b : Int) :
  a = id (a * b^2) := sorry
```
where it would say it could not synthesize an `HMul Int Int Nat` instance. The issue in this case is that `HPow` uses default instances, so while the expression tree elaborator decides that `a * b^2` should be referring to an `Int`, the actual elaborated type is temporarily a metavariable, so when the binrel elaborator is looking at both sides of the equality, it decides that `Nat` will work and coercions don't need to be inserted.

The fix is to have the expression tree elaborator elaborate all the operators using the computed `maxType` as the expected type.
src/Lean/Elab/Extra.lean Outdated Show resolved Hide resolved
@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 May 18, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request May 18, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request May 18, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan label May 18, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented May 18, 2024

Mathlib CI status (docs):

@kmill kmill marked this pull request as draft May 18, 2024 17:49
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request May 18, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request May 18, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added builds-mathlib CI has verified that Mathlib builds against this PR and removed breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan labels May 18, 2024
@kmill kmill marked this pull request as ready for review May 18, 2024 20:05
@kmill kmill added the awaiting-review Waiting for someone to review the PR label May 18, 2024
@leodemoura leodemoura removed the awaiting-review Waiting for someone to review the PR label May 18, 2024
@leodemoura leodemoura added this pull request to the merge queue May 18, 2024
Merged via the queue into leanprover:master with commit b639d10 May 18, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builds-mathlib CI has verified that Mathlib builds against this PR 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.

None yet

3 participants