Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

[Merged by Bors] - fix(solve_by_elim): handle multiple goals with different hypotheses #4519

Closed
wants to merge 4 commits into from

Conversation

semorrison
Copy link
Collaborator

@semorrison semorrison commented Oct 8, 2020

Previously solve_by_elim* would operate on multiple goals (only succeeding if it could close all of them, and performing backtracking across goals), however it would incorrectly only use the local context from the main goal. If other goals had different sets of hypotheses, ... various bad things could happen!

This PR arranges so that solve_by_elim* inspects the local context later, so it picks up the correct hypotheses.


@bryangingechen bryangingechen added the t-meta Tactics, attributes or user commands label Oct 8, 2020
@semorrison semorrison added the WIP Work in progress label Oct 8, 2020
@semorrison semorrison added awaiting-review The author would like community review of the PR and removed WIP Work in progress labels Oct 17, 2020
@@ -55,7 +68,7 @@ As an optimisation, after we build the list of `tactic expr`s, we actually run t
that do not in fact produce metavariables with a simple `return` tactic.
-/
meta def mk_assumption_set (no_dflt : bool) (hs : list simp_arg_type) (attr : list name) :
tactic (list (tactic expr)) :=
tactic (list (tactic expr) × tactic (list expr)) :=
Copy link
Member

Choose a reason for hiding this comment

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

This type is definitely tricky to digest. I guess it's okay for now, considering the documentation. If it were any more complicated, I'd suggest bundling it into a structure assumption_set or something.

@robertylewis
Copy link
Member

bors merge

@github-actions github-actions bot added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed awaiting-review The author would like community review of the PR labels Oct 17, 2020
bors bot pushed a commit that referenced this pull request Oct 18, 2020
…4519)

Previously `solve_by_elim*` would operate on multiple goals (only succeeding if it could close all of them, and performing backtracking across goals), however it would incorrectly only use the local context from the main goal. If other goals had different sets of hypotheses, ... various bad things could happen!

This PR arranges so that `solve_by_elim*` inspects the local context later, so it picks up the correct hypotheses.



Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
@bors
Copy link

bors bot commented Oct 18, 2020

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title fix(solve_by_elim): handle multiple goals with different hypotheses [Merged by Bors] - fix(solve_by_elim): handle multiple goals with different hypotheses Oct 18, 2020
@bors bors bot closed this Oct 18, 2020
@bors bors bot deleted the solve_by_elim_multiple branch October 18, 2020 04:29
b-mehta pushed a commit that referenced this pull request Oct 20, 2020
…4519)

Previously `solve_by_elim*` would operate on multiple goals (only succeeding if it could close all of them, and performing backtracking across goals), however it would incorrectly only use the local context from the main goal. If other goals had different sets of hypotheses, ... various bad things could happen!

This PR arranges so that `solve_by_elim*` inspects the local context later, so it picks up the correct hypotheses.



Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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.

3 participants