Skip to content

[Merged by Bors] - fix: update context in recursive calls in split_ifs#761

Closed
dwrensha wants to merge 3 commits intomasterfrom
fix-split-ifs
Closed

[Merged by Bors] - fix: update context in recursive calls in split_ifs#761
dwrensha wants to merge 3 commits intomasterfrom
fix-split-ifs

Conversation

@dwrensha
Copy link
Copy Markdown
Member

Fixes #760.

Before this change, we were failing at an fvar lookup here:

let typ ← instantiateMVars (← inferType (mkFVar fvarId))

@dwrensha dwrensha requested a review from kim-em November 28, 2022 03:02
@dwrensha
Copy link
Copy Markdown
Member Author

I'm kind of confused about why this existing testcase works without this change:

example (x : Nat) (p q : Prop) [Decidable p] [Decidable q] :
x = if p then (if q then x else x) else x := by
split_ifs
· rfl
· rfl
· rfl

@dwrensha
Copy link
Copy Markdown
Member Author

Ah, the difference is that the new testcase puts the nested ifs within a hypothesis. So if I update the old testcase like this:

example (x : Nat) (p : Prop) [Decidable p]
    (h : x = if (if p then False else True) then y else y)
  : x = y := by
  split_ifs at h
  -- does not fully split

Then we get the same bad behavior (before we apply this PR).

@kim-em
Copy link
Copy Markdown
Contributor

kim-em commented Nov 28, 2022

bors merge

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review labels Nov 28, 2022
@bors
Copy link
Copy Markdown

bors bot commented Nov 28, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title fix: update context in recursive calls in split_ifs [Merged by Bors] - fix: update context in recursive calls in split_ifs Nov 28, 2022
@bors bors bot closed this Nov 28, 2022
@bors bors bot deleted the fix-split-ifs branch November 28, 2022 16:13
rosborn added a commit that referenced this pull request Nov 29, 2022
* master: (26 commits)
  docs (Order.BoundedOrder): fix typos (#775)
  feat: port Algebra.Order.Monoid.Cancel.Defs (#774)
  feat: port Order.Disjoint (#773)
  feat: port linarith (#526)
  feat: port Algebra.Order.Monoid.Defs (#771)
  feat: port control.functor (#612)
  feat(Algebra/GroupWithZero/Commute): port file (#762)
  feat: port Logic.Equiv.Option (#674)
  chore: fix todos in `to_additive` (#765)
  feat(Algebra/Order/Monoid/MinMax): port file (#763)
  fix: update context in recursive calls in split_ifs (#761)
  feat(Algebra/Regular/Basic): port file (#758)
  feat: port Order.BoundedOrder (#697)
  feat: port Data.Pi.Algebra (#564)
  feat: port Algebra.Hom.Embedding (#764)
  fix: to_additive generates equation lemmas for target (#767)
  fix: fix translation errors in various files (#716)
  fix: remove submodules (#766)
  feat(Algebra/Group/Commute): port file (#750)
  feat(Algebra/Ring/Commute): port file (#759)
  ...
bors bot pushed a commit that referenced this pull request Dec 8, 2022
Now that #761 has landed, we can use `split_ifs` in the proof of `setValue`.
The proof is still not as nice as it could be, because we don't have `cc` yet.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge This PR has been sent to bors.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

split_ifs doesn't completely split hypotheses

2 participants