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] - feat(tactic/interactive/choose): nondependent choose #3806

Closed
wants to merge 10 commits into from

Conversation

digama0
Copy link
Member

@digama0 digama0 commented Aug 15, 2020

Now you can write choose! to eliminate propositional arguments from the chosen functions.

example (h : ∀n m : ℕ, n < m → ∃i j, m = n + i ∨ m + j = n) : true :=
begin
  choose! i j h using h,
  guard_hyp i := ℕ → ℕ → ℕ,
  guard_hyp j := ℕ → ℕ → ℕ,
  guard_hyp h := ∀ (n m : ℕ), n < m → m = n + i n m ∨ m + j n m = n,
  trivial
end

@sgouezel
Copy link
Collaborator

I see you are using inhabited, but nonempty would be more general, and I don't think the junk value used matters much.

Also, what about using just choose for the new version, instead of choose!?

@digama0
Copy link
Member Author

digama0 commented Aug 15, 2020

The reason for not making it default is to avoid the hit for typeclass search. But I can switch it to use nonempty instead (dunno how that affects the idea that was floated to maybe choose what the default value / function is).

@robertylewis robertylewis added the awaiting-review The author would like community review of the PR label Aug 15, 2020
@sgouezel
Copy link
Collaborator

I would'nt be too much worried about performance issues, as it's an interactive tactic and it's not used that frequently, so one additional typeclass search from time to time shouldn't be that bad. But I get your point, so let's keep the ! unless other people say that they'd also like it as the default.

And thanks for switching to nonempty!

@PatrickMassot
Copy link
Member

I just checked that it works great on two real life examples I'm interested in (including #3740). My only little complain is that when choose! cannot find the relevant nonempty instance, it silently falls back to the choose behavior without error message.

@PatrickMassot PatrickMassot added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR labels Aug 16, 2020
Copy link
Member

@robertylewis robertylewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just a couple doc comments. Maybe it would be better to move this out of tactic.core/tactic.interactive into its own file.

src/logic/function/basic.lean Outdated Show resolved Hide resolved
src/tactic/core.lean Outdated Show resolved Hide resolved
@digama0 digama0 added awaiting-review The author would like community review of the PR and removed awaiting-author A reviewer has asked the author a question or requested changes labels Aug 16, 2020
@robertylewis robertylewis added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR labels Aug 16, 2020
digama0 and others added 2 commits August 16, 2020 09:44
Co-authored-by: Rob Lewis <Rob.y.lewis@gmail.com>
@robertylewis
Copy link
Member

bors merge

@bors
Copy link

bors bot commented Aug 16, 2020

👎 Rejected by label

@github-actions github-actions bot added the ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) label Aug 16, 2020
@robertylewis robertylewis removed the awaiting-author A reviewer has asked the author a question or requested changes label Aug 16, 2020
@robertylewis
Copy link
Member

bors merge

bors bot pushed a commit that referenced this pull request Aug 16, 2020
Now you can write `choose!` to eliminate propositional arguments from the chosen functions.
```
example (h : ∀n m : ℕ, n < m → ∃i j, m = n + i ∨ m + j = n) : true :=
begin
  choose! i j h using h,
  guard_hyp i := ℕ → ℕ → ℕ,
  guard_hyp j := ℕ → ℕ → ℕ,
  guard_hyp h := ∀ (n m : ℕ), n < m → m = n + i n m ∨ m + j n m = n,
  trivial
end
```
@bors
Copy link

bors bot commented Aug 16, 2020

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(tactic/interactive/choose): nondependent choose [Merged by Bors] - feat(tactic/interactive/choose): nondependent choose Aug 16, 2020
@bors bors bot closed this Aug 16, 2020
@bors bors bot deleted the choose_nondep branch August 16, 2020 20:22
@bryangingechen bryangingechen added the t-meta Tactics, attributes or user commands label Feb 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) t-meta Tactics, attributes or user commands
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants