Skip to content
This repository was archived by the owner on Mar 24, 2022. It is now read-only.

move lucet-validate into lucetc crate#575

Merged
pchickey merged 10 commits into
mainfrom
pch/fold_validate_in
Aug 19, 2020
Merged

move lucet-validate into lucetc crate#575
pchickey merged 10 commits into
mainfrom
pch/fold_validate_in

Conversation

@pchickey
Copy link
Copy Markdown
Contributor

@pchickey pchickey commented Aug 7, 2020

lucet-validate was creating more trouble as a separate crate than it was worth. And, to fix some design problems, I ended up rewriting it piece-by-piece.

  • lucet-validate crate is gone. What is left of it and its tests are merged into lucetc. This eliminates a dev-dependencies loop that caused us problems with cargo publish.
  • lucetc::validate now uses a builder interface for construction, which is more flexible than before. It supports both wasi commands and reactors, though we don't do anything with reactors at the moment (lucet-runtime doesnt know what to do with them yet).
  • a Validator, once constructed, gets its register_import and register_export methods called by the ModuleInfo struct's ModuleEnvironment impl. This means that as cranelift_wasm::translate_module parses the wasm module, we are validating each function import and export. Previously, lucet-validate had a ModuleType struct that was constructed by a totally separate parsing pass on the wasm module; this was never a great design, it was just the quick and dirty version.
  • Rather than fail on the first error, a Validator collects all of its errors internally. After translate_module is complete, we get the whole set of validation errors out of Validator and, if there are any, report them all and stop compilation.
  • Tests were added for the new behavior.

The PR is arranged as a series of commits that transform the lucet-validate code into the final form step by step, with tests passing at each commit, if you want to read it that way.

This PR closes #438: now all validation errors are reported, with positive and negative tests.

@pchickey pchickey marked this pull request as ready for review August 13, 2020 23:53
@cratelyn cratelyn self-requested a review August 19, 2020 17:05
cratelyn
cratelyn previously approved these changes Aug 19, 2020
Copy link
Copy Markdown
Member

@cratelyn cratelyn left a comment

Choose a reason for hiding this comment

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

Great work! Thank you 🙂

Copy link
Copy Markdown
Member

@cratelyn cratelyn left a comment

Choose a reason for hiding this comment

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

♻️ ✔️

@pchickey pchickey merged commit b9c4fb2 into main Aug 19, 2020
@pchickey pchickey deleted the pch/fold_validate_in branch August 19, 2020 20:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants