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

simp doesn't rewrite in definitions because of unassigned metavariables #4613

Open
3 tasks done
fpvandoorn opened this issue Jul 1, 2024 · 5 comments
Open
3 tasks done
Labels
bug Something isn't working P-medium We may work on this issue if we find the time

Comments

@fpvandoorn
Copy link
Contributor

Prerequisites

Please put an X between the brackets as you perform the following steps:

Description

The following code fails:

set_option trace.Meta.Tactic.simp true
def foo {ι : Type _} (n m : ι) (h h' : n = m) : ℕ := by
  simp only [id h] at h' -- no progress
  simp only [h] at h' -- works

The trace message is

[Meta.Tactic.simp.rewrite] id h:1000, has unassigned metavariables after unification 

Note that the unassigned metavariable is the universe metavariable of ι

Expected behavior: simp only [id h] should succeed, just like simp only [h] already does.

Related Issues

#4398 is basically the same issue, and #4493 didn't really fix the underlying issue.

Impact

Add 👍 to issues you consider important. If others are impacted by this issue, please ask them to add 👍 to it.

@fpvandoorn fpvandoorn added the bug Something isn't working label Jul 1, 2024
@leodemoura
Copy link
Member

leodemoura commented Jul 1, 2024

#4398 is basically the same issue, and #4493 didn't really fix the underlying issue.

The change in #4482 is only applied to defs whose type is a Prop. This is not the case here.
We can easily remove the <- isProp condition there, and eliminate the discrepancy in the def and theorem elaborators.
See comment at #4482 for potential impact on users.

@fpvandoorn
Copy link
Contributor Author

I don't think it's feasible to generalize universe metavariables in definitions before elaborating the value. As mentioned in #4482, that will lead to huge inconveniences.

I don't think that the rule "don't have universe metavariables around" is the right solution here. Here is a (somewhat contrived) example using theorem.

theorem bar (n m : Nat) : n = m → n = m := by
  have : ∀ (α : Type _) (x y : α), x = y → x = y := by
    intros α x y h
    simp only [id h] -- error. The proof works if you remove `id`
  exact this Nat n m

I might be completely misunderstanding why the behavior here happens, but my understanding is as follows:
My understanding of this issue is that when t is an expression (not just a name), then simp [t] will refuse to rewrite with t if t still contains universe metavariables. Maybe the check can be "no universe metavariables that are introduced by this simp" (if this can be checked cheaply)?

Feel free to mark this as low priority/wontfix, this is a low priority issue. I was surprised when I encountered it (it was within a def), so I opened an issue.

@leodemoura
Copy link
Member

@fpvandoorn Note that the theorem bar example in your message will be rejected in the future. See #4610

@leodemoura
Copy link
Member

I don't think that the rule "don't have universe metavariables around" is the right solution here

The motivation is simplicity.

@fpvandoorn
Copy link
Contributor Author

Ok, this happens only rarely in Mathlib, so I think the fallout will be limited.

After that I think the only issue is in definitions.

@leanprover-bot leanprover-bot added the P-medium We may work on this issue if we find the time label Aug 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P-medium We may work on this issue if we find the time
Projects
None yet
Development

No branches or pull requests

3 participants