-
Notifications
You must be signed in to change notification settings - Fork 470
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
fix: WF.Fix: deduplicate subsumed goals before running tactic #3024
Conversation
before code like def dup (a : Nat) (b : Nat := a) := a + b def rec : Nat → Nat | 0 => 1 | n+1 => dup (dup (dup (rec n))) decreasing_by decreasing_tactic would run the `decreasing_tactic` 8 tims, because the recursive call `rec n` gets duplicate due to the default paramter. Similar effects can be observed due to dependent types or tactics like `cases`. This is wasteful, and is confusing to the user when they use `decreasing_by` interactively. Therfore, we now go through the proof obligations (MVars) and if solving one would imply solving another one, we assign the mvars to each other accordingly. This PR is a sibling of #3004.
I’ll run this against mathlib once 9290b49 is tagged as |
|
Got mathlib perf results, nothing to see. http://speed.lean-fro.org/mathlib4/compare/6d60f603-6faa-4727-9a04-42abb15e1477/to/535315ad-b4ab-4d6a-b3fd-67e8b2ff12c7 There is an “open Mathlib — wall-clock 7.8%” but I think it’s a fluke: It doesn't show up when comparing this branch with the latest |
before code like
would run the
decreasing_tactic
8 tims, because the recursive callrec n
gets duplicate due to the default paramter. Similar effects canbe observed due to dependent types or tactics like
cases
.This is wasteful, and is confusing to the user when they use
decreasing_by
interactively. Therfore, we now go through the proofobligations (MVars) and if solving one would imply solving another one,
we assign the mvars to each other accordingly.
This PR is a sibling of #3004.