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

chore(parser): remove recursion in field parsing, fixes #514 #519

Merged
merged 7 commits into from
Apr 13, 2023

Conversation

goto-bus-stop
Copy link
Member

The selection::selection_set parser already supports parsing multiple fields, so it looks like this recursion is unnecessary. Tests pass right now but I'll also run the fuzzer for a bit to make sure.

Copy link
Member

@lrlna lrlna left a comment

Choose a reason for hiding this comment

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

lgtm

@lrlna
Copy link
Member

lrlna commented Apr 11, 2023

how is the fuzzer? is it happy?

@goto-bus-stop
Copy link
Member Author

I forgot about it and it accidentally ran overnight so I'm pretty sure it's happy lol

Just need to tweak some of the tests here that expect the old counting!

@goto-bus-stop
Copy link
Member Author

Heh I couldn't get to the bottom of the issue where we continued parsing after a recursion limit was reached, but I think I just put the check in the wrong place yesterday. Got it right with fresh eyes this morning 😇

@lrlna
Copy link
Member

lrlna commented Apr 13, 2023

excellllllent ^_^

@goto-bus-stop goto-bus-stop merged commit eacadd5 into main Apr 13, 2023
@goto-bus-stop goto-bus-stop deleted the field-norecur branch April 13, 2023 09:51
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.

2 participants