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

Tidy up serialization layer #154

Closed
wants to merge 85 commits into from
Closed

Tidy up serialization layer #154

wants to merge 85 commits into from

Conversation

chriso
Copy link
Contributor

@chriso chriso commented Jul 5, 2024

This PR tidies up the serialization layer.

A goal of this work was to patch our graph traversal code so that we could improve error messages when something goes wrong. Unfortunately this proved to be difficult because we had 5 (!) different places where we were traversing the graph of values/pointers. I'll address the error messages in a follow-up PR and instead focus on tidying up the code as a prerequisite. We're now using the visitor pattern so that we only have one implementation of the graph traversal. I've fixed a handful of bugs while consolidating this code.

Another goal was to reduce our usage of unsafe (pointer casting / arithmetic / dereferencing) as much as possible, and instead lean on Go reflection to do the heavy lifting. I moved all generic reflection code into a new internal/reflectext (reflection extension) package, to make the serialization layer clearer. There are still places where we need to step outside the safety of the reflection API, but I've consolidated this all into internal/reflectext/unsafe.go and documented why.

@chriso chriso marked this pull request as ready for review July 5, 2024 14:29
@chriso chriso closed this Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant