Skip to content

Add a CodeBuilder type to the wasmtime crate#8181

Merged
alexcrichton merged 5 commits intobytecodealliance:mainfrom
alexcrichton:module-builder
Mar 20, 2024
Merged

Add a CodeBuilder type to the wasmtime crate#8181
alexcrichton merged 5 commits intobytecodealliance:mainfrom
alexcrichton:module-builder

Conversation

@alexcrichton
Copy link
Member

@alexcrichton alexcrichton commented Mar 19, 2024

This commit is extracted from #8055 and adds a new CodeBuilder type which is intended to be able to further configure compilation beyond what the constructors of Module already provide. For example in #8055 knobs will be added to process *.dwp files for more debuginfo processing.

This commit is extracted from bytecodealliance#8055 and adds a new `ModuleBuilder` type
which is intended to be able to further configure compilation beyond
what the constructors of `Module` already provide. For example in bytecodealliance#8055
knobs will be added to process `*.dwp` files for more debuginfo
processing.

Co-authored-by: yowl00 <scott.waye@hubse.com>
@alexcrichton alexcrichton requested a review from a team as a code owner March 19, 2024 16:51
@alexcrichton alexcrichton requested review from fitzgen and removed request for a team March 19, 2024 16:51
@alexcrichton
Copy link
Member Author

I'll note that while I've called this ModuleBuilder it actually supports components as well, so there's probably a better name for this.

@github-actions github-actions bot added the wasmtime:api Related to the API of the `wasmtime` crate itself label Mar 19, 2024
@github-actions
Copy link

Subscribe to Label Action

cc @peterhuene

Details This issue or pull request has been labeled: "wasmtime:api"

Thus the following users have been cc'd because of the following labels:

  • peterhuene: wasmtime:api

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

LGTM!

As far as naming goes, the only alternatives I can think of are

  • ModuleOrComponentBuilder
  • CodeBuilder

Or maybe splitting this type in two in the public API (and sharing an underlying impl) so that we have both a ModuleBuilder and a ComponentBuilder.

Comment on lines +18 to +19
/// WebAssembly module. Less advanced configuration can use constructors such
/// as:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// WebAssembly module. Less advanced configuration can use constructors such
/// as:
/// WebAssembly module. Most configuration can use simple constructors such
/// as:

Copy link
Member

Choose a reason for hiding this comment

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

(Don't want to create a weird "advanced" vs not thing where people who consider themselves Serious Wasmtime Users will want to use this without actually having a good reason. Might be overthinking things here.)

@alexcrichton
Copy link
Member Author

It feels kind of nice being able to configure components/modules in one builder since it's all wasm anyway so I think I'll stick with one for now vs copying APIs, but I'll rename to CodeBuilder as I like that name more.

@alexcrichton alexcrichton changed the title Add a ModuleBuilder type to the wasmtime crate Add a CodeBuilder type to the wasmtime crate Mar 20, 2024
@alexcrichton alexcrichton enabled auto-merge March 20, 2024 19:39
@alexcrichton alexcrichton added this pull request to the merge queue Mar 20, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 20, 2024
@alexcrichton alexcrichton enabled auto-merge March 20, 2024 20:36
@alexcrichton alexcrichton added this pull request to the merge queue Mar 20, 2024
Merged via the queue into bytecodealliance:main with commit 55664f5 Mar 20, 2024
@alexcrichton alexcrichton deleted the module-builder branch March 20, 2024 21:18
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Mar 28, 2024
This commit fixes a mistake in bytecodealliance#8181 which meant that the caching for
components was no longer working. The mistake is fixed in this commit as
well as a new test being added too.
github-merge-queue bot pushed a commit that referenced this pull request Mar 28, 2024
This commit fixes a mistake in #8181 which meant that the caching for
components was no longer working. The mistake is fixed in this commit as
well as a new test being added too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wasmtime:api Related to the API of the `wasmtime` crate itself

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants