Skip to content

Commit 2ba365e

Browse files
committed
fix(TacticAnalysis): remove syntax range check (#28950)
In #28802 we discovered the tactic analysis framework does not fire on declarations using `where` clauses. The reason is that `def foo where ...` gets turned into `def foo := { ... }`, and the `{ ... }` syntax is assigned a range outside of the `where ...` syntax; this causes the syntax range check to omit the whole declaration body. Moreover, [Verso involves pieces of syntax without any range at all](https://leanprover.zulipchat.com/#narrow/channel/287929-mathlib4/topic/Possible.20Verso.E2.80.93Mathlib.20conflict/with/543352839). So the range of a piece of syntax is essentially meaningless and we should not check it at all. Co-authored-by: Anne C.A. Baanen <vierkantor@vierkantor.com>
1 parent 4f582f1 commit 2ba365e

File tree

2 files changed

+18
-11
lines changed

2 files changed

+18
-11
lines changed

Mathlib/Tactic/TacticAnalysis.lean

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -145,19 +145,14 @@ would result in three sequences:
145145
Similarly, a declaration with multiple `by` blocks results in each of the blocks getting its
146146
own sequence.
147147
-/
148-
def findTacticSeqs (stx : Syntax) (tree : InfoTree) :
148+
def findTacticSeqs (tree : InfoTree) :
149149
CommandElabM (Array (Array (ContextInfo × TacticInfo))) := do
150-
let some enclosingRange := stx.getRange? |
151-
throw (Exception.error stx m!"operating on syntax without range")
152150
-- Turn the CommandElabM into a surrounding context for traversing the tree.
153151
let ctx ← read
154152
let state ← get
155153
let ctxInfo := { env := state.env, fileMap := ctx.fileMap, ngen := state.ngen }
156154
let out ← tree.visitM (m := CommandElabM) (ctx? := some ctxInfo)
157-
(fun _ i _ => do
158-
if let some range := i.stx.getRange? then
159-
pure <| enclosingRange.start <= range.start && range.stop <= enclosingRange.stop
160-
else pure false)
155+
(fun _ _ _ => pure true) -- Assumption: a tactic can occur as a child of any piece of syntax.
161156
(fun ctx i _c cs => do
162157
let relevantChildren := (cs.filterMap id).toArray
163158
let childTactics := relevantChildren.filterMap Prod.fst
@@ -190,16 +185,15 @@ def findTacticSeqs (stx : Syntax) (tree : InfoTree) :
190185

191186
/-- Run the tactic analysis passes from `configs` on the tactic sequences in `stx`,
192187
using `trees` to get the infotrees. -/
193-
def runPasses (configs : Array Pass) (stx : Syntax)
194-
(trees : PersistentArray InfoTree) : CommandElabM Unit := do
188+
def runPasses (configs : Array Pass) (trees : PersistentArray InfoTree) : CommandElabM Unit := do
195189
let opts ← getLinterOptions
196190
let enabledConfigs := configs.filter fun config =>
197191
-- This can be `none` in the file where the option is declared.
198192
if let some opt := config.opt then getLinterValue opt opts else false
199193
if enabledConfigs.isEmpty then
200194
return
201195
for i in trees do
202-
for seq in (← findTacticSeqs stx i) do
196+
for seq in (← findTacticSeqs i) do
203197
for config in enabledConfigs do
204198
config.run seq
205199

@@ -217,7 +211,7 @@ def tacticAnalysis : Linter where run := withSetOptionIn fun stx => do
217211
let env ← getEnv
218212
let configs := (tacticAnalysisExt.getState env).2
219213
let trees ← getInfoTrees
220-
runPasses configs stx trees
214+
runPasses configs trees
221215

222216
initialize addLinter tacticAnalysis
223217

MathlibTest/TacticAnalysis.lean

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,19 @@ example : x = z := by
3838
rw [xy]
3939
rw [yz]
4040

41+
-- Definitions using `where` clauses did not get picked up by the framework,
42+
-- since apparently their syntax bounds do not match the original.
43+
structure Fact (p : Prop) : Prop where
44+
out : p
45+
/--
46+
warning: Try this: rw [xy, yz]
47+
-/
48+
#guard_msgs in
49+
example : Fact (x = z) where
50+
out := by
51+
rw [xy]
52+
rw [yz]
53+
4154
end rwMerge
4255

4356
section mergeWithGrind

0 commit comments

Comments
 (0)