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(library/type_context): inconsistent unification of match-based definitions #141

Merged
merged 1 commit into from
Mar 8, 2020

Conversation

Vierkantor
Copy link
Collaborator

The issue was that nat.add didn't get unified with λ a b, nat.add a b, because nat.add is defined using matches. If such a definition is applied to a term, only when the term matches one of the match cases the whole is unfolded. In contrast, if there is no application, the constant nat.add is happily unfolded all the way. Thus the elaborator decides unification fails.

There are basically two solutions: unfolding more (i.e. also unfold nat.add a b) or unfolding less (i.e. don't unfold nat.add). Since unfolding nat.add would expose implementation details, namely the nat.add._main function, choosing to unfold less is better.

But this leads to another issue: now λ <a, b>, a does not unify with λ <a, b>, a if they are defined separately. This is not a big issue since example : let c := 0 in (λ <a, b>, a + c) = (λ <a, b>, a + c) := rfl didn't work anyway. Hence the changed tests.

The issue was that `nat.add` didn't get unified with `λ a b, nat.add a b`,
because `nat.add` is defined using matches. If such a definition is applied to
a term, only when the term matches one of the match cases the whole is
unfolded.  In contrast, if there is no application, the constant `nat.add` is
happily unfolded all the way. Thus the elaborator decides unification fails.

There are basically two solutions: unfolding more (i.e. also unfold `nat.add a
b`) or unfolding less (i.e. don't unfold `nat.add`). Since unfolding `nat.add`
would expose implementation details, namely the `nat.add._main` function,
choosing to unfold less is better.

But this leads to another issue: now `λ <a, b>, a` does not unify with
`λ <a, b>, a` if they are defined separately. This is not a big issue since
`example : let c := 0 in (λ <a, b>, a + c) = (λ <a, b>, a + c) := rfl`
didn't work anyway. Hence the changed tests.
@gebner gebner added this to the Lean 3.7 milestone Mar 5, 2020
@gebner
Copy link
Member

gebner commented Mar 8, 2020

@Vierkantor I don't remember: did you test this change on mathlib? Were there any issues?

@Vierkantor
Copy link
Collaborator Author

I tested these changes together with #134, and compared to #134 alone, there were no issues. Should I test building mathlib again with all changes that were merged?

@gebner
Copy link
Member

gebner commented Mar 8, 2020

Ok, good. There's no point in testing right now, everything's broken.

@gebner gebner merged commit 9e3515a into leanprover-community:master Mar 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants