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.

Sign up for GitHub

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

Jump to bottom

Parser Refactor #627

Merged
merged 31 commits into from
Jul 5, 2023
Merged

Parser Refactor #627

merged 31 commits into from
Jul 5, 2023

Conversation

rvcas
Copy link
Member

@rvcas rvcas commented Jun 20, 2023

closes #587

@rvcas rvcas added the parsing Parser work label Jun 20, 2023
@rvcas rvcas force-pushed the rvcas/parser_refactor branch 3 times, most recently from 4e4c4d2 to b92d9f2 Compare June 28, 2023 00:17
@KtorZ KtorZ mentioned this pull request Jun 30, 2023
Closed
@KtorZ KtorZ mentioned this pull request Jul 4, 2023
Merged
@rvcas rvcas marked this pull request as ready for review July 4, 2023 15:19
@rvcas rvcas force-pushed the rvcas/parser_refactor branch 2 times, most recently from 5e09e20 to bd47731 Compare July 4, 2023 21:15
rvcas and others added 19 commits July 4, 2023 17:19
@rvcas
feat(parser): move definitions to their own modules
fc580d4
@rvcas
feat: finish moving definitions and start exprs
3339d41
@rvcas
feat: move expr_parser and remove module.rs to definitions
e3ed5d3
@rvcas
feat: start splitting apart expr_parser
9c98fc8
@rvcas
feat: move anon fn, let, and expect
eea94fc
@cfcosta @rvcas
feat: add insta as dependency
58c854f 
We are going to start to add "golden"/snapshot tests, so we are using
[insta](https://insta.rs) to do so.
@cfcosta @rvcas
feat: add first code snapshot test with insta
eeaa1a0
@cfcosta @rvcas
chore: add more details on snapshot tests
63cdb8a 
We added a macro to add more information about the code that is being
tested, so we can add lots and lots of small snapshot tests.
@rvcas
feat: finish splitting up parsers
2226747
@rvcas
feat: move some token processing to the lexer
f878ef7
@cfcosta @rvcas
chore: refactor all parse tests to use assert_parse!
291dedf
@rvcas
test: add empty list test
8a7df7f
@rvcas
test: adjust snapshots
da0b969
@rvcas
test: assert_module
7157527
@rvcas
test: add indoc to assert_expr macro
f9c099a
@rvcas
test(parser): int list
44d0432
@rvcas
test(parser): list spread
baf807c
@rvcas
test(parser): rename definitions to definition and more tests
6b05d6a
@rvcas
test(parser): move over the validator tests and some misc tests to pa…
bd8c13c 
…rser
KtorZ
KtorZ approved these changes Jul 5, 2023
View reviewed changes
crates/aiken-lang/src/parser/expr/anonymous_binop.rs Outdated Show resolved Hide resolved
crates/aiken-lang/src/parser/expr/assignment.rs Outdated Show resolved Hide resolved
crates/aiken-lang/src/parser/expr/block.rs Show resolved Hide resolved
crates/aiken-lang/src/parser/expr/bytearray.rs Outdated Show resolved Hide resolved
crates/aiken-lang/src/parser/expr/if_else.rs Outdated Show resolved Hide resolved
crates/aiken-lang/src/parser/expr/if_else.rs Show resolved Hide resolved
crates/aiken-lang/src/parser/expr/mod.rs Outdated Show resolved Hide resolved
crates/aiken-lang/src/parser/expr/mod.rs Outdated Show resolved Hide resolved
crates/aiken-lang/src/parser/expr/mod.rs Outdated Show resolved Hide resolved
crates/aiken-lang/src/parser/utils.rs Outdated Show resolved Hide resolved
@KtorZ
Copy link
Member

KtorZ commented Jul 5, 2023

Nice work, that was desperately needed. It now makes me eager to go in each parser, one after the other to improve them and get some nice backtracking errors.

I've left a few comments which I don't mind addressing myself or in a pairing session. I'll be on Discord streaming, feel free to join :)

KtorZ and others added 7 commits July 5, 2023 13:57
@KtorZ
Replace 'public' utils with a more generic 'optional_flag'
93e010b 
  The 'public' util was arguably not really adding much except a layer of indirection.
  In the end, one useful parsing behavior to abstract is the idea of 'optional flag' that we use for both 'pub' and 'opaque' keywords.
@KtorZ
Favor pattern-match over if-else when parsing assignment kinds
44eb501 
Equality on a union-type is potentially dangerous as the compiler won't
complain if we add a new case that we don't cover. Reversing the
assignment by yielding a `Token` for a given `AssignmentKind`. This way
we can use a pattern-match that got us covered for future cases.
@KtorZ
Move 'utils::bytearray' to 'expr/bytearray'
e15cdaf
@KtorZ
Move parsing of literals under new 'literal' parser module group
66296df 
  Also moved the logic for 'int' and 'string' there though it is trivial. Yet, for bytearray, it tidies things nicely by removing them from the 'utils' module.
@KtorZ
rename: 'r' → 'expression' & 'seq_r' → 'sequence'
4f6defc 
Better readability.
@KtorZ
Move chain and chained parsing into their own submodule
a306d6e 
Alleviate a bit more the top-level expression parser. Note that we
probably need a bit more disciplined in what we export and at what level
because there doesn't seem to be much logic as for whether a parser is
private, exported to the crate only or to the wide open. I'd be in favor
of exporting everything by default.
@rvcas
chore: clippy fix
e331b34
@KtorZ KtorZ merged commit 82dc795 into main Jul 5, 2023
@KtorZ KtorZ deleted the rvcas/parser_refactor branch July 5, 2023 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KtorZ KtorZ KtorZ approved these changes

Assignees
No one assigned
Labels
parsing Parser work
Projects
Project Tracking
Status: 🚀 Released
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Parser Refactor
3 participants
@rvcas @KtorZ @cfcosta