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

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

Closed
wants to merge 3 commits into from

Conversation

dwrensha
Copy link
Member

Fixes #760.

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

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

@dwrensha
Copy link
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
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).

@semorrison
Copy link
Contributor

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

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