Skip to content

Commit b9fc957

Browse files
committed
fix(unfold?): improved implementation of isUserFriendly (#31608)
This PR fixes `isUserFriendly`, in `unfold?`, for two reasons: - We should only worry about bad constants in the expression if they appear in the visible part of the expression. They should be allowed in e.g. instance implicit arguments. - We don't want to get raw projections, because these are generally not what we want to work with.
1 parent 26b1003 commit b9fc957

File tree

2 files changed

+35
-7
lines changed

2 files changed

+35
-7
lines changed

Mathlib/Tactic/Widget/InteractiveUnfold.lean

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ are not presented in the list of suggested rewrites.
4747
This is implemented with `unfoldProjDefaultInst?`.
4848
4949
Additionally, we don't want to unfold into expressions involving `match` terms or other
50-
constants marked as `Name.isInternalDetail`. So all such results are filtered out.
51-
This is implemented with `isUserFriendly`.
50+
constants marked as `Name.isInternalDetail`, and we don't want raw projections.
51+
So, all such results are filtered out. This is implemented with `isUserFriendly`.
5252
5353
-/
5454

@@ -106,13 +106,24 @@ where
106106
fun _ =>
107107
return acc
108108

109-
/-- Determine whether `e` contains no internal names. -/
110-
def isUserFriendly (e : Expr) : Bool :=
111-
!e.foldConsts (init := false) (fun name => (· || name.isInternalDetail))
109+
/-- Determine whether `e` contains no internal names or raw projections.
110+
We only consider the explicit parts of `e`, because it may happen that an
111+
instance implicit argument is marked as an internal detail, but that is not a problem. -/
112+
partial def isUserFriendly (e : Expr) : MetaM Bool := do
113+
match e with
114+
| .const name _ => return !name.isInternalDetail
115+
| .proj .. => return false
116+
| .app .. =>
117+
e.withApp fun f args => do
118+
(isUserFriendly f) <&&> do
119+
let finfo ← getFunInfoNArgs f e.getAppNumArgs
120+
e.getAppNumArgs.allM fun i _ =>
121+
if finfo.paramInfo[i]?.all (·.isExplicit) then isUserFriendly args[i]! else return true
122+
| _ => return true
112123

113124
/-- Return the consecutive unfoldings of `e` that are user friendly. -/
114-
def filteredUnfolds (e : Expr) : MetaM (Array Expr) :=
115-
return (← unfolds e).filter isUserFriendly
125+
def filteredUnfolds (e : Expr) : MetaM (Array Expr) := do
126+
(← unfolds e).filterM isUserFriendly
116127

117128
end InteractiveUnfold
118129

MathlibTest/interactiveUnfold.lean

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,3 +111,20 @@ info: Unfolds for fun x => id x:
111111
-/
112112
#guard_msgs in
113113
#unfold? fun x => id x
114+
115+
-- We don't want to get the suggestion `inst✝.toMulOneClass.toMul.1 a a` because it isn't useful:
116+
variable {α : Type} [Group α] (a : α) in
117+
/-- info: No unfolds found for a * a -/
118+
#guard_msgs in
119+
#unfold? a * a
120+
121+
-- The proof `aux._proof_1` is an implementation detail. It should not be a problem if
122+
-- that appears in the expression, as long as it appears inside an implicit argument.
123+
def aux {α : Type} [Semiring α] := (3 : α)
124+
/--
125+
info: Unfolds for 3 + 3:
126+
· Nat.add 3 3
127+
· 6
128+
-/
129+
#guard_msgs in
130+
#unfold? 3 + @OfNat.ofNat _ _ (@instOfNatAtLeastTwo Nat (nat_lit 3) inferInstance aux._proof_1)

0 commit comments

Comments
 (0)