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

Customize the PEGTL parse_tree implementation #252

Merged
merged 13 commits into from
May 18, 2022

Conversation

wravery
Copy link
Contributor

@wravery wravery commented May 17, 2022

Pending taocpp/PEGTL#309 (or perhaps taocpp/PEGTL#313), I need to suppress the try/catch/rethrow handling in match_control_unwind to unblock reasonably deep nested selection sets on Windows. This was originally reported as #222, which was mitigated by adding a depth limit parameter, but it still needs to be set to a very low number to avoid a stack overflow on Windows. Since ast_node does not have any unwind handling, it is safe to let the exception unwind directly from where it is thrown to the point where we're attempting to parse a GraphQL document. If/when that PR is merged, I can revert to the original version in PEGTL.

This also simplifies a lot of the ast_node implementation by inheriting from PEGTL's parse_tree::basic_node CRTP type. This will help when migrating to the 4.x/main branch of PEGTL because some of the input iterator types have been replaced with frobnicator types, indicating that they should be considered a low-level internal feature and not something that should be referenced outside of it.

If for some reason taocpp/PEGTL#313 is merged without taocpp/PEGTL#309, then I will need to make some more changes before I can revert the custom parse tree. By inheriting from parse_tree::basic_node, I'm exposing an unwind implementation on ast_node. I will likely need to compose the parse_tree::basic_node as a private member instead of inheriting it to be able to suppress the call to unwind with taocpp/PEGTL#313 by itself.

@wravery wravery merged commit b8edf5f into microsoft:main May 18, 2022
@wravery wravery deleted the custom-parse-tree branch May 18, 2022 00:04
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.

None yet

1 participant