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

Adds Module.SectionSize and constrains text parsing rules #231

Merged
merged 3 commits into from
Feb 14, 2022

Conversation

codefromthecrypt
Copy link
Contributor

This consolidates more code around SectionID, notably consistently
getting and using section ID length. This also enforces rules in text
parsing such as up to one memory, and imported functions can't be
defined after regular ones.

This consolidates more code around SectionID, notably consistently
getting and using section ID length. This also enforces rules in text
parsing such as up to one memory, and imported functions can't be
defined after regular ones.

Signed-off-by: Adrian Cole <adrian@tetrate.io>
positionModuleExportFunc
positionModuleExportMemory
positionModuleStart
positionImport
Copy link
Contributor Author

Choose a reason for hiding this comment

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

a couple more refactors left (not now) and I'll be able to substitute sectionID for the top-level parsing position, and take these out.

@@ -331,7 +361,7 @@ func (p *moduleParser) addFunctionName(name string) {
// Ex. If there is no signature `(import "" "main" (func))`
// calls onImportFunc here ---^
func (p *moduleParser) parseImportFunc(tok tokenType, tokenBytes []byte, line, col uint32) (tokenParser, error) {
idx := wasm.Index(len(p.module.ImportSection))
idx := p.module.SectionSize(wasm.SectionIDImport)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have in-flight work to make an import parser. Due to this module.SectionSize function, I can decouple the parser from needing a reference to the module via a function type like... func(SectionID) uint32

Copy link
Member

@mathetake mathetake left a comment

Choose a reason for hiding this comment

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

looks fine except the inconsistency on custom sections

@codefromthecrypt
Copy link
Contributor Author

ps there's another need not addressed here, which is count by second index. Ex.

  • custom sections by subsection ID
  • imports/exports by kind

I didn't address this, yet but I think at some point we can refactor things so that you can for example, tell the function index namespace count w/o a for loop.

Signed-off-by: Adrian Cole <adrian@tetrate.io>
@mathetake
Copy link
Member

SectionItemCounts would be exact? :D

@codefromthecrypt
Copy link
Contributor Author

Thanks for the notes on CustomSection @mathetake it is messy and seems per-spec it is allowed to repeat by name also. I'm inclined to drop support for it and just skip anything not the name section (in another PR)

@codefromthecrypt codefromthecrypt merged commit 25591e0 into main Feb 14, 2022
@codefromthecrypt codefromthecrypt deleted the section-size branch February 14, 2022 00:41
r8d8 pushed a commit to r8d8/wazero that referenced this pull request Feb 14, 2022
…s#231)

This consolidates more code around SectionID, notably consistently
getting and using section ID length. This also enforces rules in text
parsing such as up to one memory, and imported functions can't be
defined after regular ones.

Signed-off-by: Adrian Cole <adrian@tetrate.io>
Signed-off-by: r8d8 <ckryvomaz@gmail.com>
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