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

matching on an if _ then _ else _ gets reduced to Decidable.rec by simp only, which it then can't further simplify #5388

Closed
3 tasks done
alexkeizer opened this issue Sep 18, 2024 · 1 comment · Fixed by #5419
Labels
bug Something isn't working P-medium We may work on this issue if we find the time

Comments

@alexkeizer
Copy link
Contributor

alexkeizer commented Sep 18, 2024

Prerequisites

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

Description

See the following MWE:

example (k : Prop)  (h : k) [Decidable k] : 
    let ⟨a, b⟩ := if k then (1, 0) else (0, 1)
    a = 1 := by
  simp [h]
  /-
  unsolved goals
  k : Prop
  h : k
  inst✝ : Decidable k
  ⊢ (Decidable.rec (fun h => (0, 1)) (fun h => (1, 0)) inst✝).fst = 1
  -/

When we match on the result of an if _ then _ else _ (recalling that let ⟨a, b⟩ := foo; ... desugars to match foo with | ⟨a, b⟩ => ...), then even simp only will already reduce this to an application of Decidable.rec.

This is unfortunate, because our simpset is not equiped to simplify Decidable.rec, even if the if-condition is known (as it is in the example), and even if we include a proof that the if-condition is true, this reduction seems to happen first, blocking any further simplification and preventing a seemingly trivial goal from being closed.

Context

This showed up in LNSym, for example, in our semantics for a sub instruction

Steps to Reproduce

MWE:

example (k : Prop)  (h : k) [Decidable k] : 
    let ⟨a, b⟩ := if k then (1, 0) else (0, 1)
    a = 1 := by
  simp [h]
  /-
  unsolved goals
  k : Prop
  h : k
  inst✝ : Decidable k
  ⊢ (Decidable.rec (fun h => (0, 1)) (fun h => (1, 0)) inst✝).fst = 1
  -/

Expected behavior: I would've expected the goal to be automatically closed by simp, seeing that we gave it a proof that the condition of the if is true

Actual behavior: simp is unable to close the goal, getting stuck with

/-
  unsolved goals
  k : Prop
  h : k
  inst✝ : Decidable k
  ⊢ (Decidable.rec (fun h => (0, 1)) (fun h => (1, 0)) inst✝).fst = 1
  -/

Versions

4.12.0-nightly-2024-09-18, at https://live.lean-lang.org/#project=lean-nightly&codez=KYDwhgtgDgNsAEAKA1vAXPACgJwPZQEp4kALdeZIgbQBFgBjASwBMwAjOCgXXIChjicAC7xAF+RgANPDaBL8nQBeeIwBmFeEJLAAdkgCMUgAxFgMAM4JEBqboL8BYeIt0LpATzunG0eFRJc7APQAtHYArlqmuDAAbsDM8ADmuGBmdqgYOPh2ZBjIdowRQoC45OR0TKwcCHnEgEVESGUs7HAAdNgMSMrh8GTyAHxIVvA2RIidOj39iPrwRkQFpsUEzcoLjkN2QQG8dgDEwNEp8AAywGBazbHYnrhaAMpC2AUJQA

Impact

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

@alexkeizer alexkeizer added the bug Something isn't working label Sep 18, 2024
bollu added a commit to leanprover/LNSym that referenced this issue Sep 19, 2024
This works around leanprover/lean4#5388,
and removes uses of `simp (config := { ground := true }) ..`.
@leanprover-bot leanprover-bot added the P-medium We may work on this issue if we find the time label Sep 20, 2024
shigoel added a commit to leanprover/LNSym that referenced this issue Sep 20, 2024
### Issues:

This works around leanprover/lean4#5388, and
removes uses of `simp (config := { ground := true }) ..`. This makes our
proof states cleaner, since we no longer see proof states involving
`Decidable.rec ...`. Peeled from #168

### Testing:

What tests have been run? Did `make all` succeed for your changes? Was
conformance testing successful on an Aarch64 machine?

`make all` succeeded, conformance succeeded.

### License:

By submitting this pull request, I confirm that my contribution is
made under the terms of the Apache 2.0 license.

---------

Co-authored-by: Shilpi Goel <[email protected]>
leodemoura added a commit that referenced this issue Sep 23, 2024
…en reducibility setting is `.reducible`

closes #5388
github-merge-queue bot pushed a commit that referenced this issue Sep 23, 2024
…en reducibility setting is `.reducible` (#5419)

closes #5388

See updated comment for additional details.
@alexkeizer
Copy link
Contributor Author

Thanks for the quick fix!

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
2 participants