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

Divide the byte compiler #1808

Closed
Razican opened this issue Jan 29, 2022 · 10 comments
Closed

Divide the byte compiler #1808

Razican opened this issue Jan 29, 2022 · 10 comments
Assignees
Labels
E-Easy Easy good first issue Good for newcomers Internal Category for changelog vm Issues and PRs related to the Boa Virtual Machine.
Milestone

Comments

@Razican
Copy link
Member

Razican commented Jan 29, 2022

Currently, the byte compiler is written in a file with almost 2,000 lines. Each execution path should be divided in submodules, in order to reduce the size of the file and to make it more maintainable.

@Razican Razican added vm Issues and PRs related to the Boa Virtual Machine. Internal Category for changelog labels Jan 29, 2022
@Razican Razican added this to the v0.15.0 milestone Mar 18, 2022
@Razican Razican added good first issue Good for newcomers E-Easy Easy labels Mar 18, 2022
@a1is43ras4
Copy link

@Razican can I take a deal with the issue?

@jasonwilliams
Copy link
Member

jasonwilliams commented Apr 11, 2022

@justunkn sure, assigned

@Razican
Copy link
Member Author

Razican commented Sep 7, 2022

@justunkn do you have any news on this?

@e-codes-stuff
Copy link
Contributor

Does this issue still need to be worked on? It appears that #2343 fixes it, I would be willing to attempt some work on it otherwise though.

@jedel1043
Copy link
Member

Does this issue still need to be worked on? It appears that #2343 fixes it, I would be willing to attempt some work on it otherwise though.

It doesn't fix it though. The byte compiler and the VM are in separate modules

@e-codes-stuff
Copy link
Contributor

Ah okay, in that case I'll see if I can do anything wrt fixing up the byte compiler organization. I'm assuming it makes the most sense to extract each of the longer match arm paths into separate files/functions?

@jedel1043
Copy link
Member

jedel1043 commented Oct 27, 2022

Yep! Thank you!

@e-codes-stuff
Copy link
Contributor

Quick question: Within the bytecompiler module, theres some code that feels confusing to read because of the ambiguity about which Literal type the code is using at any given time, currently this is resolved by explicitly using the path to the ast::expression::literal::Literal type, would it make sense for me to use it as AstLiteral or something more easily understood as different from the bytecompiler::Literal type?

@jedel1043
Copy link
Member

Quick question: Within the bytecompiler module, theres some code that feels confusing to read because of the ambiguity about which Literal type the code is using at any given time, currently this is resolved by explicitly using the path to the ast::expression::literal::Literal type, would it make sense for me to use it as AstLiteral or something more easily understood as different from the bytecompiler::Literal type?

AstLiteral sounds good!

@Razican
Copy link
Member Author

Razican commented Nov 2, 2022

I think this happens in multiple places, where we have the parser and the AST node with the same time. Maybe we should have a review on all these names.

@bors bors bot closed this as completed in ce51449 Dec 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-Easy Easy good first issue Good for newcomers Internal Category for changelog vm Issues and PRs related to the Boa Virtual Machine.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants