Skip to content

Commit e96f033

Browse files
committed
refactor(TacticAnalysis): use full CommandElabM context in test (#29771)
Running the `rwMerge` linter on the whole of Mathlib reveals that the tactic analysis framework did not always correctly pass the environment to the test step of a pass. By running the test in a full `CommandElabM` and adding a `ContextInfo` and `TacticInfo`, we can access the right environment (using the `ctxI.runTacticCode` function). There are still a few subtle issues to do with notation but I would like to get this fix in first, since it involves some larger refactors. Co-authored-by: Anne C.A. Baanen <vierkantor@vierkantor.com>
1 parent f84aa9a commit e96f033

File tree

4 files changed

+47
-18
lines changed

4 files changed

+47
-18
lines changed

Mathlib/Tactic/TacticAnalysis.lean

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ structure ComplexConfig where
288288
-/
289289
trigger (context : Option ctx) (currentTactic : Syntax) : TriggerCondition ctx
290290
/-- Code to run in the context of the tactic, for example an alternative tactic. -/
291-
test (context : ctx) (goal : MVarId) : MetaM out
291+
test (ctxI : ContextInfo) (i : TacticInfo) (context : ctx) (goal : MVarId) : CommandElabM out
292292
/-- Decides what to report to the user. -/
293293
tell (stx : Syntax) (originalSubgoals : List MVarId) (originalHeartbeats : Nat)
294294
(new : out) (newHeartbeats : Nat) : CommandElabM (Option MessageData)
@@ -311,7 +311,7 @@ def testTacticSeq (config : ComplexConfig) (tacticSeq : Array (TSyntax `tactic))
311311
if !i.mayFail then
312312
logWarning m!"original tactic '{stx}' failed: {e.toMessageData}"
313313
return [goal]
314-
let (new, newHeartbeats) ← withHeartbeats <| i.ctxI.runTactic i.tacI goal <| config.test ctx
314+
let (new, newHeartbeats) ← withHeartbeats <| config.test i.ctxI i.tacI ctx goal
315315
if let some msg ← config.tell stx oldGoals oldHeartbeats new newHeartbeats then
316316
logWarning msg
317317

Mathlib/Tactic/TacticAnalysis/Declarations.lean

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ Authors: Anne Baanen, Edward van de Meent
66
import Mathlib.Tactic.TacticAnalysis
77
import Mathlib.Tactic.ExtractGoal
88
import Mathlib.Tactic.MinImports
9-
import Lean.Elab.Tactic.Meta
109
import Lean.Elab.Command
1110

1211
/-!
@@ -25,7 +24,7 @@ private inductive TerminalReplacementOutcome where
2524
| remainingGoals (stx : TSyntax `tactic) (goals : List MessageData)
2625
| error (stx : TSyntax `tactic) (msg : MessageData)
2726

28-
open Elab.Command
27+
open Elab Command
2928

3029
/--
3130
Define a pass that tries replacing one terminal tactic with another.
@@ -41,28 +40,30 @@ for example `Mathlib.Tactic.linarith`.
4140
in `stx` and the current `goal`.
4241
-/
4342
def terminalReplacement (oldTacticName newTacticName : String) (oldTacticKind : SyntaxNodeKind)
44-
(newTactic : SyntaxMVarIdMetaM (TSyntax `tactic))
43+
(newTactic : ContextInfoTacticInfoSyntax → CommandElabM (TSyntax `tactic))
4544
(reportFailure : Bool := true) (reportSuccess : Bool := false)
4645
(reportSlowdown : Bool := false) (maxSlowdown : Float := 1) :
4746
TacticAnalysis.Config := .ofComplex {
4847
out := TerminalReplacementOutcome
4948
ctx := Syntax
5049
trigger _ stx := if stx.getKind == oldTacticKind
5150
then .accept stx else .skip
52-
test stx goal := do
53-
let tac ← newTactic stx goal
51+
test ctxI i stx goal := do
52+
let tac ← newTactic ctxI i stx
5453
try
55-
let (goals, _) ← Lean.Elab.runTactic goal tac
54+
let goals ← ctxI.runTacticCode i goal tac
5655
match goals with
5756
| [] => return .success tac
5857
| _ => do
5958
let goalsMessages ← goals.mapM fun g => do
60-
let e ← instantiateMVars (← g.getType)
59+
let e ← ctxI.runTactic i g <| fun g => do instantiateMVars (← g.getType)
6160
pure m!"⊢ {MessageData.ofExpr e}\n"
6261
return .remainingGoals tac goalsMessages
6362
catch _e =>
6463
let name ← mkAuxDeclName `extracted
65-
let ((sig, _, modules), _) ← (Mathlib.Tactic.ExtractGoal.goalSignature name goal).run
64+
-- Rerun in the original tactic context, since `omega` changes the state.
65+
let ((sig, _, modules), _) ← ctxI.runTactic i goal (fun goal =>
66+
(Mathlib.Tactic.ExtractGoal.goalSignature name goal).run)
6667
let imports := modules.toList.map (s!"import {·}")
6768
return .error tac m!"{"\n".intercalate imports}\n\ntheorem {sig} := by\n fail_if_success {tac}\n {stx}"
6869
tell stx old oldHeartbeats new newHeartbeats :=
@@ -118,7 +119,7 @@ def grindReplacementWith (tacticName : String) (tacticKind : SyntaxNodeKind)
118119
(reportFailure : Bool := true) (reportSuccess : Bool := false)
119120
(reportSlowdown : Bool := false) (maxSlowdown : Float := 1) :
120121
TacticAnalysis.Config :=
121-
terminalReplacement tacticName "grind" tacticKind (fun _ _ => `(tactic| grind))
122+
terminalReplacement tacticName "grind" tacticKind (fun _ _ _ => `(tactic| grind))
122123
reportFailure reportSuccess reportSlowdown maxSlowdown
123124

124125
end Mathlib.TacticAnalysis
@@ -148,7 +149,7 @@ register_option linter.tacticAnalysis.regressions.omegaToCutsat : Bool := {
148149
@[tacticAnalysis linter.tacticAnalysis.regressions.omegaToCutsat,
149150
inherit_doc linter.tacticAnalysis.regressions.omegaToCutsat]
150151
def omegaToCutsatRegressions :=
151-
terminalReplacement "omega" "cutsat" ``Lean.Parser.Tactic.omega (fun _ _ => `(tactic| cutsat))
152+
terminalReplacement "omega" "cutsat" ``Lean.Parser.Tactic.omega (fun _ _ _ => `(tactic| cutsat))
152153
(reportSuccess := false) (reportFailure := true)
153154

154155
/-- Report places where `omega` can be replaced by `cutsat`. -/
@@ -158,7 +159,7 @@ register_option linter.tacticAnalysis.omegaToCutsat : Bool := {
158159
@[tacticAnalysis linter.tacticAnalysis.omegaToCutsat,
159160
inherit_doc linter.tacticAnalysis.omegaToCutsat]
160161
def omegaToCutsat :=
161-
terminalReplacement "omega" "cutsat" ``Lean.Parser.Tactic.omega (fun _ _ => `(tactic| cutsat))
162+
terminalReplacement "omega" "cutsat" ``Lean.Parser.Tactic.omega (fun _ _ _ => `(tactic| cutsat))
162163
(reportSuccess := true) (reportFailure := false)
163164

164165
/-- Suggest merging two adjacent `rw` tactics if that also solves the goal. -/
@@ -174,14 +175,18 @@ def Mathlib.TacticAnalysis.rwMerge : TacticAnalysis.Config := .ofComplex {
174175
match stx with
175176
| `(tactic| rw [$args,*]) => .continue ((ctx.getD #[]).push args)
176177
| _ => if let some args := ctx then if args.size > 1 then .accept args else .skip else .skip
177-
test ctx goal := withOptions (fun opts => opts.set `grind.warning false) do
178+
test ctxI i ctx goal := do
178179
let ctxT : Array (TSyntax `Lean.Parser.Tactic.rwRule) := ctx.flatten.map (⟨·⟩)
179180
let tac ← `(tactic| rw [$ctxT,*])
181+
let oldMessages := (← get).messages
180182
try
181-
let (goals, _) ← Lean.Elab.runTactic goal tac
183+
let goals ← ctxI.runTacticCode i goal tac
182184
return (goals, ctxT.map (↑·))
183185
catch _e => -- rw throws an error if it fails to pattern-match.
184186
return ([goal], ctxT.map (↑·))
187+
finally
188+
-- Drop any messages, since they will appear as if they are genuine errors.
189+
modify fun s => { s with messages := oldMessages }
185190
tell _stx _old _oldHeartbeats new _newHeartbeats := pure <|
186191
if new.1.isEmpty then
187192
m!"Try this: rw {new.2}"
@@ -351,11 +356,11 @@ def Mathlib.TacticAnalysis.introMerge : TacticAnalysis.Config := .ofComplex {
351356
-- if `intro` is used without arguments, treat it as `intro _`
352357
<| if args.size = 0 then #[⟨mkHole x⟩] else args)
353358
| _ => if let some args := ctx then if args.size > 1 then .accept args else .skip else .skip
354-
test ctx goal := do
359+
test ctxI i ctx goal := do
355360
let ctxT := ctx.flatten
356361
let tac ← `(tactic| intro $ctxT*)
357362
try
358-
let _ ← Lean.Elab.runTactic goal tac
363+
let _ ← ctxI.runTacticCode i goal tac
359364
return some tac
360365
catch _e => -- if for whatever reason we can't run `intro` here.
361366
return none

Mathlib/Tactic/Tauto.lean

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ register_option linter.tacticAnalysis.tautoToGrind : Bool := {
231231
@[tacticAnalysis linter.tacticAnalysis.tautoToGrind,
232232
inherit_doc linter.tacticAnalysis.tautoToGrind]
233233
def tautoToGrind :=
234-
terminalReplacement "tauto" "grind" ``Mathlib.Tactic.Tauto.tauto (fun _ _ => `(tactic| grind))
234+
terminalReplacement "tauto" "grind" ``Mathlib.Tactic.Tauto.tauto (fun _ _ _ => `(tactic| grind))
235235
(reportSuccess := true) (reportFailure := false)
236236

237237
/-- Debug `grind` by identifying places where it does not yet supersede `tauto`. -/

MathlibTest/TacticAnalysis.lean

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,30 @@ example : Fact (x = z) where
5151
rw [xy]
5252
rw [yz]
5353

54+
universe u
55+
56+
def a : PUnit.{u} := ⟨⟩
57+
def b : PUnit.{u} := ⟨⟩
58+
def c : PUnit.{u} := ⟨⟩
59+
theorem ab : a = b := rfl
60+
theorem bc : b = c := rfl
61+
62+
/--
63+
warning: Try this: rw [ab.{u}, bc.{u}]
64+
-/
65+
#guard_msgs in
66+
example : a.{u} = c := by
67+
rw [ab.{u}]
68+
rw [bc.{u}]
69+
70+
theorem xyz (h : x = z → y = z) : x = y := by rw [h yz]; rfl
71+
72+
-- The next example tripped up `rwMerge` because `rw [xyz fun h => ?_, ← h, xy]` gives
73+
-- an unknown identifier `h`.
74+
example : x = y := by
75+
rw [xyz fun h => ?_]
76+
rw [← h, xy]
77+
5478
end rwMerge
5579

5680
section mergeWithGrind

0 commit comments

Comments
 (0)