Skip to content

Fix Wasmtime C++ API without wat feature.#12706

Merged
dicej merged 1 commit intobytecodealliance:mainfrom
PiotrSikora:wat
Mar 4, 2026
Merged

Fix Wasmtime C++ API without wat feature.#12706
dicej merged 1 commit intobytecodealliance:mainfrom
PiotrSikora:wat

Conversation

@PiotrSikora
Copy link
Contributor

No description provided.

@PiotrSikora PiotrSikora requested a review from a team as a code owner March 3, 2026 07:33
@PiotrSikora PiotrSikora requested review from dicej and removed request for a team March 3, 2026 07:33
@github-actions github-actions bot added wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime:c-api Issues pertaining to the C API. labels Mar 3, 2026
Copy link
Contributor

@dicej dicej left a comment

Choose a reason for hiding this comment

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

Thanks! I take it CI didn't cover this case, correct? Would you mind adding a line to e.g. here which covers the scenario to ensure this doesn't regress in the future?

@PiotrSikora
Copy link
Contributor Author

Thanks! I take it CI didn't cover this case, correct?

Correct.

Would you mind adding a line to e.g. here which covers the scenario to ensure this doesn't regress in the future?

There are actually 2 separate cases here (bundled into single PR per contributing guidelines against trivial changes), that I've run into within a few days from each other, and neither would be covered by those tests.

  1. The C++ API issue requires linking C++ code against Wasmtime built without wat feature enabled (i.e. running test_capi job with cmake -S examples -B examples/build [...] -DWASMTIME_FEATURE_WAT=OFF and then building the examples). I don't mind adding this, but it seems quite costly in terms of the CPU time, and it would target only this single feature, so in practice we'd need to add the whole matrix if we want to be exhaustive.

  2. The unused variable case could be detected with cargo check build with default-features = false, features = ["cranelift", "compile-time-builtins"], but detecting issues like that would require building with all possible combinations, unless we want to target this very specific case.

Let me know how to proceed. Thanks!

@dicej
Copy link
Contributor

dicej commented Mar 3, 2026

Let me know how to proceed. Thanks!

Thanks for the explanation. Let's add the additional cargo check but not the cmake job; I agree that the extra CPU time of the cmake one is not worth it.

Signed-off-by: Piotr Sikora <code@piotrsikora.dev>
@PiotrSikora PiotrSikora changed the title Fix build without wat feature. Fix Wasmtime C++ API without wat feature. Mar 4, 2026
@PiotrSikora
Copy link
Contributor Author

Thanks for the explanation. Let's add the additional cargo check but not the cmake job; I agree that the extra CPU time of the cmake one is not worth it.

Actually, I'm going to split this into C++ API fixes (this PR) and other feature-related build fixes (#12719) after all.

Since we don't want to add cmake check, this one should be good to go. Thanks!

@dicej dicej added this pull request to the merge queue Mar 4, 2026
@PiotrSikora
Copy link
Contributor Author

Oops! I forgot to push the version with the unrelated build fix removed. Luckily, I caught it before it was merged. Not sure if you need to re-queue it. Sorry for the noise!

Merged via the queue into bytecodealliance:main with commit 7a4f53a Mar 4, 2026
@PiotrSikora PiotrSikora deleted the wat branch March 5, 2026 17:14
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 wasmtime:c-api Issues pertaining to the C API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants