Skip to content

Commit 72d7d46

Browse files
Khaclaude
andcommitted
fix: only persist context-free type class resolution cache entries
Results with abstracted metavariables are only valid relative to the elaboration context that created them: degrees of freedom not determined by the cache key (e.g. universe metavariables of intermediate instances, cf. `Small`) are resolved by ambient unification constraints. Persisting such entries and reusing them in a different context (a later command, or a different elaboration phase of the same declaration) produces incorrectly instantiated terms: in `Mathlib.Condensed.Discrete.Module`, a cached `(sheafToPresheaf _ _).IsRightAdjoint` result was reused with universe instantiations from the wrong context, yielding kernel-rejected declarations. The cache is now split into two tiers: the persistent environment extension only receives entries with a metavariable-free key and a closed result, which are context-free (free universe parameters and fvar references are pinned by the key). All other entries go to a reintroduced transient `Meta.Cache.synthInstance` tier with the previous per-`Meta.State` lifetime and semantics, the scope for which such sharing was originally designed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 9fc0dc3 commit 72d7d46

3 files changed

Lines changed: 59 additions & 31 deletions

File tree

src/Lean/Meta/Basic.lean

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -410,14 +410,18 @@ We should also investigate the impact on memory consumption.
410410
abbrev DefEqCache := PersistentHashMap DefEqCacheKey Bool
411411

412412
/--
413-
Cache datastructures for type inference, whnf, and definitional equality.
413+
Cache datastructures for type inference, type class resolution, whnf, and definitional equality.
414414
415-
The type class resolution cache is not part of this structure; it is stored in an environment
416-
extension so that it persists across commands (see `synthInstanceCacheExt`).
415+
The `synthInstance` field is only the *transient* tier of the type class resolution cache: it
416+
holds context-sensitive entries (keys containing metavariables, or results with abstracted
417+
metavariables), whose validity is tied to the current elaboration context. Context-free entries
418+
are stored in an environment extension instead so that they persist across commands (see
419+
`synthInstanceCacheExt`).
417420
-/
418421
structure Cache where
419422
inferType : InferTypeCache := {}
420423
funInfo : FunInfoCache := {}
424+
synthInstance : SynthInstanceCache := {}
421425
whnf : WhnfCache := {}
422426
defEqTrans : DefEqCache := {} -- transient cache for terms containing mvars or using nonstandard configuration options, it is frequently reset.
423427
defEqPerm : DefEqCache := {} -- permanent cache for terms not containing mvars and using standard configuration options
@@ -681,13 +685,13 @@ def resetCache : MetaM Unit :=
681685
modifyCache fun _ => {}
682686

683687
@[inline] def modifyInferTypeCache (f : InferTypeCache → InferTypeCache) : MetaM Unit :=
684-
modifyCache fun ⟨ic, c1, c2, c3, c4⟩ => ⟨f ic, c1, c2, c3, c4⟩
688+
modifyCache fun ⟨ic, c1, c2, c3, c4, c5⟩ => ⟨f ic, c1, c2, c3, c4, c5
685689

686690
@[inline] def modifyDefEqTransientCache (f : DefEqCache → DefEqCache) : MetaM Unit :=
687-
modifyCache fun ⟨c1, c2, c3, defeqTrans, c4⟩ => ⟨c1, c2, c3, f defeqTrans, c4
691+
modifyCache fun ⟨c1, c2, c3, c4, defeqTrans, c5⟩ => ⟨c1, c2, c3, c4, f defeqTrans, c5
688692

689693
@[inline] def modifyDefEqPermCache (f : DefEqCache → DefEqCache) : MetaM Unit :=
690-
modifyCache fun ⟨c1, c2, c3, c4, defeqPerm⟩ => ⟨c1, c2, c3, c4, f defeqPerm⟩
694+
modifyCache fun ⟨c1, c2, c3, c4, c5, defeqPerm⟩ => ⟨c1, c2, c3, c4, c5, f defeqPerm⟩
691695

692696
def mkExprConfigCacheKey (expr : Expr) : MetaM ExprConfigCacheKey :=
693697
return { expr, configKey := (← read).configKey }

src/Lean/Meta/Instances.lean

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,13 @@ builtin_initialize instanceExtension : SimpleScopedEnvExtension InstanceEntry In
115115
}
116116

117117
/--
118-
Cache for `synthInstance` results; see `Lean.Meta.SynthInstance`. It is stored in an environment
119-
extension so that it persists across commands; it is not stored in `.olean` files. It is
120-
registered in this module so that `addInstance` can invalidate it.
118+
Persistent tier of the `synthInstance` result cache; see `Lean.Meta.SynthInstance`. It is stored
119+
in an environment extension so that it persists across commands; it is not stored in `.olean`
120+
files. It is registered in this module so that `addInstance` can invalidate it.
121+
122+
Only *context-free* entries are stored here: keys without metavariables and closed results.
123+
Context-sensitive entries (whose validity depends on the ambient metavariable context) live in
124+
the transient `Meta.Cache.synthInstance` tier instead.
121125
122126
The cache map is stored behind an `IO.Ref` (`none` only as an unreachable `Inhabited` fallback):
123127
cache *fills* mutate the ref and thus survive elaborator backtracking, like the `Meta.Cache`
@@ -127,8 +131,8 @@ environment is rolled back, e.g. when a speculatively added instance is discarde
127131
the cache entries that were computed with it.
128132
129133
Note that environment values derived from the same environment share the ref and thus the cache;
130-
in contexts that should not share the cache with their surroundings (e.g. async elaboration
131-
branches or incremental reuse across edits), it may need to be replaced explicitly in the future.
134+
this is sound for context-free entries, but e.g. incremental reuse across edits may require
135+
replacing the ref explicitly in the future.
132136
-/
133137
builtin_initialize synthInstanceCacheExt : EnvExtension (Option (IO.Ref SynthInstanceCache)) ←
134138
registerEnvExtension (some <$> IO.mkRef {}) (asyncMode := .local) -- mere cache, keep local

src/Lean/Meta/SynthInstance.lean

Lines changed: 40 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -879,20 +879,38 @@ private def applyAbstractResult? (type : Expr) (abstResult? : Option AbstractMVa
879879
check result
880880
return some result
881881

882-
/-- Returns the type class resolution cache entry for `key`; see `synthInstanceCacheExt`. -/
882+
/--
883+
Returns the type class resolution cache entry for `key` from the transient
884+
(`Meta.Cache.synthInstance`) or persistent (`synthInstanceCacheExt`) tier.
885+
-/
883886
private def findCachedResult? (key : SynthInstanceCacheKey) :
884887
MetaM (Option (Option AbstractMVarsResult)) := do
888+
if let some result? := (← get).cache.synthInstance.find? key then
889+
return some result?
885890
let some ref := synthInstanceCacheExt.getState (← getEnv) | return none
886891
return (← ref.get).find? key
887892

888893
/--
889-
Inserts a result into the type class resolution cache. The insertion mutates the cache ref
890-
instead of the environment, so it survives environment rollbacks; see `synthInstanceCacheExt`.
894+
Inserts a result into the type class resolution cache: into the persistent tier if `persist` is
895+
true, and otherwise into the transient `Meta.Cache.synthInstance` tier, which has the lifetime of
896+
the current `Meta.State`.
897+
898+
Only context-free entries may be persisted: the key must not contain metavariables and the result
899+
must be closed. Results with abstracted metavariables are only valid relative to the elaboration
900+
context that created them: their degrees of freedom (e.g. universe metavariables not determined
901+
by the key, cf. `Small`) are resolved by ambient constraints, so reusing them in a different
902+
context can produce incorrectly instantiated terms.
903+
904+
Persistent insertions mutate the cache ref instead of the environment, so they survive
905+
environment rollbacks; see `synthInstanceCacheExt`.
891906
-/
892-
private def insertCachedResult (key : SynthInstanceCacheKey) (result? : Option AbstractMVarsResult) :
893-
MetaM Unit := do
894-
let some ref := synthInstanceCacheExt.getState (← getEnv) | return ()
895-
ref.modify (·.insert key result?)
907+
private def insertCachedResult (key : SynthInstanceCacheKey) (result? : Option AbstractMVarsResult)
908+
(persist : Bool) : MetaM Unit := do
909+
if persist then
910+
let some ref := synthInstanceCacheExt.getState (← getEnv) | return ()
911+
ref.modify (·.insert key result?)
912+
else
913+
modifyCache fun c => { c with synthInstance := c.synthInstance.insert key result? }
896914

897915
/--
898916
Auxiliary function for converting a cached `AbstractMVarsResult` returned by `SynthInstance.main` into an `Expr`.
@@ -914,19 +932,21 @@ private def applyCachedAbstractResult? (type : Expr) (abstResult? : Option Abstr
914932

915933
/-- Helper function for caching synthesized type class instances. -/
916934
private def cacheResult (cacheKey : SynthInstanceCacheKey) (kind : PreprocessKind) (abstResult? : Option AbstractMVarsResult) (result? : Option Expr) : MetaM Unit := do
917-
-- **TODO**: simplify this function.
918-
match abstResult? with
919-
| none => insertCachedResult cacheKey none
920-
| some abstResult =>
921-
if abstResult.numMVars == 0 && abstResult.paramNames.isEmpty && kind matches .noMVars | .mvarsNoOutputParams then
922-
match result? with
923-
| none => insertCachedResult cacheKey none
924-
| some result =>
925-
-- See `applyCachedAbstractResult?` If new metavariables have **not** been introduced,
926-
-- we don't need to perform extra checks again when reusing result.
927-
insertCachedResult cacheKey (some { expr := result, paramNames := #[], mvars := #[] })
928-
else
929-
insertCachedResult cacheKey (some abstResult)
935+
-- The stored value: for a closed result we store the concrete `result` expr with an empty
936+
-- `AbstractMVarsResult` so that `applyCachedAbstractResult?` can skip re-`check`ing it.
937+
let value? :=
938+
match abstResult? with
939+
| none => none
940+
| some abstResult =>
941+
if abstResult.numMVars == 0 && abstResult.paramNames.isEmpty && kind matches .noMVars | .mvarsNoOutputParams then
942+
result?.map fun result => { expr := result, paramNames := #[], mvars := #[] }
943+
else
944+
some abstResult
945+
-- Only context-free entries may be persisted: mvar-free key (`.noMVars`) and a closed value
946+
-- (no abstracted metavariables); see `insertCachedResult`.
947+
let persist := kind matches .noMVars &&
948+
(value?.all fun r => r.numMVars == 0 && r.paramNames.isEmpty)
949+
insertCachedResult cacheKey value? (persist := persist)
930950

931951
def synthInstanceCore? (type : Expr) (maxResultSize? : Option Nat := none) : MetaM (Option Expr) := do
932952
let opts ← getOptions

0 commit comments

Comments
 (0)