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

Semicolon as statement ender #7

Merged
merged 13 commits into from
Jul 6, 2023
Merged

Conversation

imteekay
Copy link
Owner

@imteekay imteekay commented May 2, 2023

Changes

  • Refactored some logic into separate functions: here and here
  • Updated the baselines: commit
  • Parsing statements until it gets to the EOF: commit
  • Add more tests: commit

Approach

  • Rather than parsing statements with the semicolon as the separator, we can parse all statements until it gets to the EOF.
  • When parsing, if it gets to the semicolon (and it's not an empty statement**), the parser skips the semicolon and starts parsing the next statement
  • I also decided to return the EOF node similar to how it works in TS: example below
source code ast
Screen Shot 2023-05-02 at 16 35 56 Screen Shot 2023-05-02 at 16 36 00

@imteekay imteekay self-assigned this May 2, 2023
src/check.ts Outdated Show resolved Hide resolved
src/types.ts Outdated Show resolved Hide resolved
src/parse.ts Outdated
const list = [element()];
while (separator()) {
while (lexer.token() !== Token.EOF) {
Copy link

Choose a reason for hiding this comment

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

I think this function should be the one checking for semicolons, not parseStatement.
Although it would be best to pass in a predicate like separator, except named terminator or something.

Copy link
Owner Author

@imteekay imteekay May 16, 2023

Choose a reason for hiding this comment

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

@sandersn Not sure if I got the exercise right this time. imteekay/mini-typescript@b74b6fc

The whole idea was

  • Add a predicate function (terminator) to handle semicolons
  • after a semicolon token, we can have
    • another semicolon: I considered that an empty statement (Allow var to have multiple declarations mini-typescript#2)
      • just added a peek function to see the next token and if it's indeed a semicolon, call the parse statement to get the EmptyStatement node
    • the rest of the tokens (string, number, eof): just parse a statement

Does that make sense? Maybe it can be improved for this particular case.

Choose a reason for hiding this comment

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

Here's the way Typescript does it, as I recall:

const list = []
while (peek()) {
  list.push(element())
  terminator()
}

where peek returns true if the current token could be the start of a new Statement, and terminator doesn't consume a token if terminators are optional. This relies on the first token of statements to be distinguishable from from non-statements, but the only non-statement that can come after a statement list is Token.EOF, so peek is just going to be () => lexer.token !== Token.EOF

This way is a bit simpler than yours, and generalises to other kinds of lists, like parameter lists, arrays, etc.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Simplified it, the implementation seems much better now! ✨ imteekay/mini-typescript@cad1fec

@imteekay imteekay requested a review from sandersn May 16, 2023 01:58
@imteekay imteekay changed the title Parse statements until EOF Semicolon as statement ender May 20, 2023
@imteekay imteekay force-pushed the semicolon-as-statement-ender branch from 3582543 to 0bfa6da Compare May 20, 2023 12:34
@imteekay imteekay merged commit 463b43c into main Jul 6, 2023
@imteekay imteekay deleted the semicolon-as-statement-ender branch July 6, 2023 10: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.

2 participants