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

refactor: Avoid copying Span when evaluating rules #1242

Merged

Conversation

audunhalland
Copy link
Contributor

As mentioned in #1209 (comment), making Spanning generic over Sp allows avoiding copying 6 usizes (the Span) in places where a borrowed Spanning needs to be type mapped. I.e. the Document contains Spanning<InputValue>, but InputValue is an enum, so when the spanned item's type gets mapped into a reference to each variant, there's no need to transform &Spanning<T, Span> to Spanning<&U, Span>. It can instead be transformed to a Spanning<&U, &Span>.

This applies to the Visitor trait and its methods related to the visitation of InputValues. The Visitor trait is public, but #[doc(hidden)].

@tyranron tyranron added enhancement Improvement of existing features or bugfix k::performance Related to performance k::refactor Refactoring, technical debt elimination and other improvements of existing code base labels Jan 29, 2024
@tyranron tyranron added this to the 0.16.0 milestone Jan 29, 2024
Copy link
Member

@tyranron tyranron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@audunhalland thanks!

@tyranron tyranron merged commit 8a69e14 into graphql-rust:master Jan 29, 2024
173 checks passed
@audunhalland audunhalland deleted the borrowed-spanning-in-visitor branch January 29, 2024 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement of existing features or bugfix k::performance Related to performance k::refactor Refactoring, technical debt elimination and other improvements of existing code base
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants