-
Notifications
You must be signed in to change notification settings - Fork 38
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
In Context-0.2, try and simplify grammar walking order #424
Comments
Are you referring to needing to traverse the models section differently than the AST? The root AST decoder we wrote for the V8 implementation uses a fairly simple DFS traversal in IDL/field order, and shouldn't require two stacks. |
Well, according to Dominic's document, it's definitely not a simple DFS. I'll use the word "enqueue" because "push" is overloaded in both Dominic's document and the Python implementation.
Did you find a way to merge stack 1 and stack 2? If my memory serves, in the Python implementation, stack 2 is hidden by the Python stack, but that's not a good practice in production code, at least for a browser, as it it makes it hard to fail softly in case memory is exceeded. Here, grammars are simple, so both stacks are always relatively small, but that's generally a risk that we want to avoid (and it might even get rejected in future versions of Firefox by the static checker). |
Ideally, I'd prefer a way to load the tables without needing to walk the grammar itself, as this is some fairly heavy machinery. In previous versions of the format, we had a convention that tables were written in the order in which they were needed for reading. This makes reading the prelude much simpler (and theoretically easier to parallelize with reading the contents), but doesn't scale well towards laziness. |
The grammar walking order in Context 0.1 is a bit complicated: it combines two depth-first strategies and implementing it properly seems to require two stacks. There is probably a simpler order that we could adopt (either breadth first or depth first or by need).
The text was updated successfully, but these errors were encountered: