-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat: elements #99
feat: elements #99
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't had the opportunity to review everything, but I've left some comments that you can ponder over tomorrow. Some are nitpicks, some are design choices.
I'd like to also highlight the fact that, at some point, we'll need to rework the constant expressions to be more expansive. It requires some linker (aka import/export) interfaces, though. But let's do this in anotther PR
I'll come back tomorrow/the day after tomorrow with more nitpicks and anything else relevant
This is on your repo so I can't add a commit to it. As such, we'll do it 90's style and give you the patch file to apply, if you find it correct |
ed6468e
to
89b9679
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more nitpick. The code under src/core/reader/types/element.rs seems to do more than just parsing.
I now understand why you were asking about WasmReader in the lass call.
All code under core/reader(/types) is for parsing, no validation or execution
All code under execution/ is for .... execution
All code under validation/ is for validation
This is my (implicit) understanding of the code structure. From what I can the element.rs file contains more than that - it also does some validation. I would suggest we split it up - we first parse what there is to parse (no validation), and under validation we verify what we parsed. Similar to globals.
3fcea9e
to
7b56da5
Compare
afdf88b
to
dbca01a
Compare
I have disabled testsuite automatic testing because we... panic |
Signed-off-by: nerodesu017 <[email protected]> Signed-off-by: nerodesu017 <[email protected]>
dbca01a
to
3700ad1
Compare
Signed-off-by: nerodesu017 <[email protected]>
0a46a2d
to
7f6597e
Compare
Pull Request Overview
This pull request adds support for elements and tables.
Testing Strategy
This pull request was tested using the provided tests which are ported from the official testsuite.
TODO or Help Wanted
This pull request still needs your help so that I can make it production-ready.
Formatting
cargo fmt
cargo check
cargo build
cargo doc
nix fmt
Github Issue
This pull request partially implements #2