Resolver: Arc<dyn Fn> with new(impl Fn) + with_resolver constructors - #16
Open
mwaddip wants to merge 3 commits into
Open
Resolver: Arc<dyn Fn> with new(impl Fn) + with_resolver constructors#16mwaddip wants to merge 3 commits into
mwaddip wants to merge 3 commits into
Conversation
…upport The Resolver type was defined as a bare function pointer (fn(&Digest32) -> Node), which cannot capture state. This makes it impossible to implement VersionedAVLStorage with a real storage backend — the resolver needs to load nodes from a database, but a function pointer cannot hold a database reference. Changed to Arc<dyn Fn(&Digest32) -> Node + Send + Sync> which allows closures that capture storage handles. Arc (not Box) because AVLTree derives Clone. Send + Sync for thread safety with concurrent readers. All 22 existing tests pass unchanged (modulo wrapping bare functions in Arc::new). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
28862a1 changed Resolver from a plain fn pointer to Arc<dyn Fn> for the persistence backend, which broke every caller written against the old signature (sigma-rust's interpreter passes bare closures at 13 sites). Take `impl Fn(&Digest32) -> Node + Send + Sync + 'static` and wrap it in the Arc inside the constructor: fn-pointer-era callers compile unchanged, capturing closures (the persistence resolver) pass straight in without their own Arc::new. Callers holding a prebuilt Resolver construct the struct literally — `resolver` is a pub field. Internal call sites (prover test + tests/common) updated to drop the now-redundant Arc::new. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
new(impl Fn) (a4a2aa7) serves closure callers — including the interpreter's bare, unannotated |digest| closures, which infer their param type only because the bound is literally Fn(&Digest32). But impl Fn rejects a pre-built Resolver (Arc<dyn Fn> does not impl Fn, E0277), and a single generic bound cannot serve both: an impl IntoResolver bound that accepts Arc breaks unannotated closures (E0282). Add a second constructor with_resolver(resolver: Resolver, ...) taking a pre-built Arc directly (no re-wrap), for storage-backend callers (the node). The interpreter keeps using new(); the node uses with_resolver(). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces #15 (closed). Gives upstream the storage-backend resolver capability without breaking the interpreter or the node, via two shape-matched constructors:
Resolver = Arc<dyn Fn(&Digest32) -> Node + Send + Sync>— the persistence need (orig Change Resolver from fn pointer to Arc<dyn Fn> for storage backends #10).new(resolver: impl Fn(&Digest32) -> Node + …)— Arc-wraps internally; lets closure callers pass bare UNANNOTATED closures (e.g. the interpreter's|digest| …` sites) with the param type inferred.with_resolver(resolver: Resolver, …)— takes a pre-built Arc directly (no re-wrap), for storage backends that capture a DB handle.A single generic constructor cannot serve both:
impl FnrejectsArc<dyn Fn>(notFn, E0277), and a bound that accepts the Arc breaks unannotated closures (E0282). All crate tests pass.