batteries icon indicating copy to clipboard operation
batteries copied to clipboard

chore: make RBNode.mem_insert more robust

Open kim-em opened this issue 2 years ago • 6 comments

This proof has three different hypotheses named h, and in a upcoming version of leanprover/lean4#2793 something is going to change in how they shadow each other.

This is a fix that works before and after. For now I'll be pointing leanprover-community/mathlib4#8076 at this branch.

@digama0 this proof has some other scary features (e.g. nonterminal simp) which we should fix at some point.

kim-em avatar Nov 01 '23 04:11 kim-em

Can you say more about how the shadowing will change? That sounds a bit scary. (Can you point me to a branch that fails without this change?)

Regarding nonterminal simp, I don't avoid it here as heavily as in mathlib, because the simp set is small(er) and stable(r), and proofs tend to involve more domain-specific notions (like the RBSet functions in this case) for which the simp set is known exactly. I think the jury is still out regarding whether this is a good idea or whether we need to be as defensive as mathlib wrt the simp set.

digama0 avatar Nov 01 '23 04:11 digama0

@digama0, try the Std branch lean-pr-testing-2793. It has no changes except to use the toolchain from leanprover/lean4#2793.

kim-em avatar Nov 01 '23 04:11 kim-em

It seems the issue is that the rw [find?_eq_zoom] at h line does not delete the original h, and the reason for this is that it results in h: root? (zoom (cmp v) t ?inr.p).fst = some v' where ?inr.p could possibly depend on the original h. The mvar is unified away in the next rewrite, but it should have been inferred from the optParam in find?_eq_zoom, and rw [find?_eq_zoom (p := .root)] at h fixes the issue. In other words, this is another manifestation of the issue you reported: https://github.com/leanprover/lean4/pull/2793#issuecomment-1788398442

digama0 avatar Nov 01 '23 05:11 digama0

Ah, great, thank you.

I'm happy in that case to either close this or merge, as you see fit!

kim-em avatar Nov 01 '23 05:11 kim-em

Let's leave it open until we find out how https://github.com/leanprover/lean4/pull/2793#issuecomment-1788398442 will be resolved.

digama0 avatar Nov 01 '23 05:11 digama0

The original upstream PR was subsumed by lean4#4596. Does this mean this PR is obsolete?

fgdorais avatar Sep 22 '24 03:09 fgdorais