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

Privatize the Lexer API #2040

Merged
merged 4 commits into from Sep 29, 2023
Merged

Privatize the Lexer API #2040

merged 4 commits into from Sep 29, 2023

Conversation

djc
Copy link
Collaborator

@djc djc commented Sep 25, 2023

Becomes an implementation detail of the Parser.

@djc djc requested a review from bluejekyll September 25, 2023 07:53
@djc djc force-pushed the lex-private branch 4 times, most recently from 6d4734f to de9481a Compare September 25, 2023 08:37
@bluejekyll
Copy link
Member

There are a bunch of changes to the state machine for the parsing, do you think you could describe some of the background there?

@djc
Copy link
Collaborator Author

djc commented Sep 26, 2023

There are a bunch of changes to the state machine for the parsing, do you think you could describe some of the background there?

Sure. I usually write commits with the intention that they can be reviewed commit-by-commit. The changes to the parser state machine are from "handle includes in the parser". Previously there were some TODOs in the lexer:

    /// TODO: it looks hacky as far we effectively duplicate parser's functionality
    /// (at least partially) and performing lexing twice.
    /// Better solution requires us to change lexer to deal
    /// with Lines-like iterator instead of String buf (or capability to combine a few
    /// lexer instances into a single lexer).
    ///
    /// TODO: $INCLUDE could specify domain name -- to support on-flight swap for Origin
    /// value we definitely need to rethink and rework loader/parser/lexer

This basically tries to address some of those issues by having the Parser hold a stack of Lexers. When an $INCLUDE is encountered, we push the stack; when a lexer runs to completion, we pop the stack. Because the parser takes its data by &str but for the case of includes we own the string inside the parser/lexer stack, the Lexer now also has to abstract over String vs &str input, which is where the CowChars type comes in.

Let me know if you have more questions.

(I wrote this code about 2 months ago so I no longer remember all the details.)

Copy link
Member

@bluejekyll bluejekyll left a comment

Choose a reason for hiding this comment

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

Everything looks good. Thanks for cleaning all of this up.

crates/proto/src/serialize/txt/zone.rs Show resolved Hide resolved
@bluejekyll bluejekyll merged commit ee530bf into main Sep 29, 2023
15 of 16 checks passed
@bluejekyll bluejekyll deleted the lex-private branch September 29, 2023 16:41
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

2 participants