Skip to content
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

Allow adapters to process types that deref to their vertex type. #481

Merged
merged 25 commits into from
Oct 21, 2023

Conversation

obi1kenobi
Copy link
Owner

@obi1kenobi obi1kenobi commented Oct 7, 2023

Before this PR, adapter implementations were unnecessarily strict: they required that the DataContext contain specifically their Vertex type. They would refuse to work in a situation where the DataContext contained a type that within it contained a Vertex i.e. could dereference to a Vertex upon request. That made it harder than necessary to expose adapters across programming languages, or use them as building blocks that can combine their functionality.

This PR changes the Adapter trait to allow any DataContext<V> representation, so long as that V type implements AsVertex<Self::Vertex> and has an appropriate lifetime for the adapter. When the adapter needs to look up the active_vertex() and get an Option<Self::Vertex> back, it can do so via the AsVertex trait which handles the conversion. AsVertex has a blanket impl for all Debug + Clone (i.e. valid Adapter::Vertex) types, so implementations should be able to upgrade to the new API by just changing the Adapter fn signatures and without implementation changes.

@obi1kenobi obi1kenobi added C-enhancement Category: raise the bar on expectations L-rust Language: affects use cases in the Rust programming language R-breaking-change Release: implementing or merging this will introduce a breaking change. R-relnotes Release: document this in the release notes of the next release labels Oct 7, 2023
trustfall_core/src/interpreter/mod.rs Outdated Show resolved Hide resolved
trustfall_core/src/interpreter/mod.rs Outdated Show resolved Hide resolved
trustfall_core/src/interpreter/mod.rs Outdated Show resolved Hide resolved
trustfall_core/src/interpreter/mod.rs Show resolved Hide resolved
trustfall_core/src/interpreter/trace.rs Outdated Show resolved Hide resolved
@obi1kenobi obi1kenobi marked this pull request as ready for review October 21, 2023 22:04
@obi1kenobi obi1kenobi enabled auto-merge (squash) October 21, 2023 22:50
@obi1kenobi obi1kenobi merged commit 722f5fa into main Oct 21, 2023
13 checks passed
@obi1kenobi obi1kenobi deleted the separate_storage_and_use_types branch October 21, 2023 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: raise the bar on expectations L-rust Language: affects use cases in the Rust programming language R-breaking-change Release: implementing or merging this will introduce a breaking change. R-relnotes Release: document this in the release notes of the next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant