-
Notifications
You must be signed in to change notification settings - Fork 386
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
synthPendingDepth
breaks TC caching
#2522
Comments
Based on the following, it looks like "sufficiently nested" is "nested more than once": lean4/src/Lean/Meta/SynthInstance.lean Lines 762 to 765 in 13ca443
|
Here is a mathlib-free version of the above example: class Norm' (E : Type) where
norm : E → Nat
export Norm' (norm)
class TopologicalSpace' (α : Type) where
random : α -- need something here for some reason
class SeminormedAddCommGroup' (α : Type) extends Norm' α, TopologicalSpace' α where
-- need to extend Norm' again for some reason; note that this doesn't seem to cause a diamond
class NonUnitalSeminormedRing' (α : Type) extends Norm' α, SeminormedAddCommGroup' α where
variable (𝕜 : Type) [NonUnitalSeminormedRing' 𝕜]
variable (E : Type) [SeminormedAddCommGroup' E]
structure ContinuousLinearMap' (M : Type) [TopologicalSpace' M] (M₂ : Type) [TopologicalSpace' M₂] where
notation:25 M " →z " M₂ => ContinuousLinearMap' M M₂
abbrev Dual : Type := E →z 𝕜
instance ContinuousLinearMap'.toSeminormedAddCommGroup' {E : Type} {F : Type}
[SeminormedAddCommGroup' E] [SeminormedAddCommGroup' F] :
SeminormedAddCommGroup' (E →z F) := sorry
def foo : E →z Dual 𝕜 (Dual 𝕜 E) := sorry
#synth SeminormedAddCommGroup' (Dual 𝕜 E) -- works fine
set_option trace.Meta.synthInstance true
example : norm (foo 𝕜 E) = 0 := by
-- this `rw` is needed to make the `have` below work
--rw [foo]
have : SeminormedAddCommGroup' (Dual 𝕜 E) := inferInstance -- fails
sorry The relevant traces are:
|
Re: "More data on impact should be collected." It was a common issue during the port that a big Scott attempted to start collecting examples in leanprover-community/mathlib4#5026 but only very late on during the porting process. I also made a more haphazard collection of some examples where |
Summary: - Take `synthPendingDepth` into account when caching TC results - Add `maxSynthPendingDepth` option with default := 2. - Add support for tracking `synthPending` failures when using `set_option diagnostics true` closes #2522 closes #3313 closes #3927 Identical to #4114 but with `maxSynthPendingDepth := 1` closes #4114 cc @semorrison
Description
It is possible to break TC caching, i.e. to create an unexpected interaction between supposedly independent elaboration steps doing the same TC query, if they occur at different depths. This is because the depth can influence the result but is not currently included in the caching logic.
Context
https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/Typeclass.20search.20is.20affected.20by.20other.20subexpressions. More data on impact should be collected, but the indirectness of the issue may make it hard for users to notice they are affected by this specific issue.
Steps to Reproduce
From Eric. Needs to be MWE'd, which should be more feasible now that we know what we need to look for (a sufficiently deep dependent TC hierarchy).
Here we first encounter (at
inclusionInDoubleDual 𝕜 E
)SeminormedAddCommGroup (Dual 𝕜 E)
as a nested TC problem, which fails because of the depth restriction. Then atinferInstance
we encounter it as a non-nested problem, which nevertheless uses the cached failure. Inserting the rewrite resets the cache and make the second problem succeed.Trace paths to the two problems without the rewrite:
Expected behavior: The rewrite should not affect the unrelated TC query.
Actual behavior: It does.
Versions
g63d2bdd490
The text was updated successfully, but these errors were encountered: