Skip to content

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Nov 17, 2025

Fix #6281

Thanks @valentynfaychuk and @moreal !

Summary by CodeRabbit

  • New Features

    • Added a new example project demonstrating RustPython compiled to WebAssembly without JavaScript dependency.
    • Included a sample WASM runtime using Wasmer with key-value store support.
  • Documentation

    • Added build and execution guides for the WASM example project.
  • Chores

    • Updated CI/CD workflow to test the reorganized WASM project structure.
    • Restructured WASM-related projects into a consolidated directory layout.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 17, 2025

Walkthrough

The PR replaces the broken wasm test infrastructure by relocating the wasm crate from wasm/wasm-unknown-test/ to a new structured example project at example_projects/wasm32_without_js/. It introduces a wasm producer crate (rustpython-without-js) and a wasm runtime consumer (wasm-runtime) using Wasmer, along with CI workflow updates.

Changes

Cohort / File(s) Summary
CI Workflow Update
.github/workflows/ci.yaml
Updated wasm test step to reference new project structure; changed working directory from wasm/wasm-unknown-test to example_projects/wasm32_without_js, adjusted cargo build manifest paths, and added execution step for the wasm runtime.
Old WASM Test Crate Removal
wasm/wasm-unknown-test/Cargo.toml, wasm/wasm-unknown-test/src/lib.rs
Deleted the broken wasm test crate entirely, removing manifest and FFI entry points (eval and __getrandom_v03_custom).
Example Project Documentation & Config
example_projects/wasm32_without_js/.gitignore, example_projects/wasm32_without_js/README.md
Added project-level gitignore patterns and comprehensive build/run documentation for the wasm32 example.
WASM Producer Crate
example_projects/wasm32_without_js/rustpython-without-js/Cargo.toml, example_projects/wasm32_without_js/rustpython-without-js/src/lib.rs
Created new cdylib crate with FFI bridge exposing eval() C function; implements Python interpreter execution via rustpython-vm and declares getrandom hook; also declares external C function stubs (kv_get, kv_put, get_code, print).
WASM Runtime Crate
example_projects/wasm32_without_js/wasm-runtime/.gitignore, example_projects/wasm32_without_js/wasm-runtime/Cargo.toml, example_projects/wasm32_without_js/wasm-runtime/README.md, example_projects/wasm32_without_js/wasm-runtime/src/main.rs
Created new Wasmer-based runtime crate with in-memory KV store backend; implements host-side FFI functions (kv_get, kv_put, print), loads and instantiates wasm module, manages memory references, and executes eval function.

Sequence Diagram

sequenceDiagram
    participant Host as Host Runtime<br>(main.rs)
    participant WasmModule as WASM Module<br>(eval)
    participant RustPython as RustPython VM
    participant KVStore as KV Store

    Host->>Host: Load WASM module from path
    Host->>Host: Initialize KV store with code
    Host->>WasmModule: Call eval(code_ptr, code_len)
    activate WasmModule
    WasmModule->>RustPython: Create Interpreter
    RustPython->>RustPython: Execute Python expr "1 + 3"
    RustPython-->>WasmModule: Result repr
    WasmModule->>Host: Call print(msg_ptr, msg_len)
    activate Host
    Host->>Host: Read string from WASM memory
    Host->>Host: Print to stdout
    Host-->>WasmModule: return 0
    deactivate Host
    WasmModule-->>Host: return 0 (success)
    deactivate WasmModule
    Host->>Host: Report result
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • WASM memory safety & pointer handling: Both producer and consumer use unsafe extern FFI with raw pointer dereference; verify boundary conditions and memory access correctness
  • Getrandom integration: Custom __getrandom_v03_custom hook implementation needs validation; ensure it doesn't break wasm runtime
  • FFI signature matching: Verify that function signatures and calling conventions align between rustpython-without-js/src/lib.rs and wasm-runtime/src/main.rs
  • Wasmer integration: Review memory environment setup and function environment exports in the runtime
  • CI workflow paths: Confirm all manifest and artifact paths resolve correctly in the new structure

Possibly related PRs

Poem

🐰 Hop, hop, a test reborn! 🌟
From broken paths, a new structure sworn,
Producer and runtime dance in tandem grace,
Wasm flows through memory's bounded space,
No more broken dreams—the bridge is strong! 🎉

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Test wasm32 without js' directly and specifically refers to the main change: adding a working test and example for wasm32 compilation without JavaScript support.
Linked Issues check ✅ Passed The PR successfully addresses issue #6281 by replacing the broken wasm-unknown-test with a functioning example and test setup that compiles and executes correctly.
Out of Scope Changes check ✅ Passed All changes are scoped to replacing the broken wasm32 test setup with a new working example and runtime, directly addressing the issue without introducing unrelated modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@youknowone youknowone force-pushed the test-wasm32-without-js branch from 555c55a to 1a92d4f Compare November 17, 2025 11:48
@youknowone youknowone marked this pull request as ready for review November 17, 2025 11:48
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 9

🧹 Nitpick comments (5)
example_projects/wasm32_without_js/wasm-runtime/.gitignore (1)

1-4: Confirm whether the !wasm/rustpython.wasm exception is still needed

This override looks like a leftover from the previous layout; the new example seems to produce rustpython-without-js/target/.../rustpython_without_js.wasm instead. If wasm/rustpython.wasm is no longer used, consider removing or updating this exception to avoid confusion.

example_projects/wasm32_without_js/rustpython-without-js/Cargo.toml (1)

13-15: Consider dropping empty [workspace] / [patch.crates-io] sections

If this crate is meant to live inside the existing top-level workspace and you’re not patching crates, these sections can be removed to reduce manifest noise and avoid accidentally creating a nested workspace.

example_projects/wasm32_without_js/wasm-runtime/README.md (1)

3-11: Clarify the wat2wasm prerequisite or remove it

The README asks to “install wat2wasm and rust”, but the example usage only shows:

cargo run --release <wasm binary>

If the current flow always consumes a .wasm produced by cargo build (as in the other README) and no .wat is involved anymore, consider either:

  • Explaining where wat2wasm fits into the workflow, or
  • Dropping the wat2wasm prerequisite to keep the instructions focused.
example_projects/wasm32_without_js/wasm-runtime/src/main.rs (2)

56-72: Clarify the intent of commented code.

The get_code function is fully commented out. Should this be removed entirely, or is it planned for future use? If it's dead code, consider removing it to improve maintainability.


95-100: Clarify hard-coded Python snippet purpose.

The runtime initializes the KV store with hard-coded Python code ("a=10;b='str';f'{a}{b}'"). For an example/demo, this might be appropriate, but consider:

  1. Documenting why this code is hard-coded
  2. Making it configurable via command-line or environment variable
  3. Adding a comment explaining what this snippet is testing
📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 916d3ba and 1a92d4f.

📒 Files selected for processing (11)
  • .github/workflows/ci.yaml (1 hunks)
  • example_projects/wasm32_without_js/.gitignore (1 hunks)
  • example_projects/wasm32_without_js/README.md (1 hunks)
  • example_projects/wasm32_without_js/rustpython-without-js/Cargo.toml (1 hunks)
  • example_projects/wasm32_without_js/rustpython-without-js/src/lib.rs (1 hunks)
  • example_projects/wasm32_without_js/wasm-runtime/.gitignore (1 hunks)
  • example_projects/wasm32_without_js/wasm-runtime/Cargo.toml (1 hunks)
  • example_projects/wasm32_without_js/wasm-runtime/README.md (1 hunks)
  • example_projects/wasm32_without_js/wasm-runtime/src/main.rs (1 hunks)
  • wasm/wasm-unknown-test/Cargo.toml (0 hunks)
  • wasm/wasm-unknown-test/src/lib.rs (0 hunks)
💤 Files with no reviewable changes (2)
  • wasm/wasm-unknown-test/src/lib.rs
  • wasm/wasm-unknown-test/Cargo.toml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-28T16:31:03.991Z
Learnt from: arihant2math
Repo: RustPython/RustPython PR: 5790
File: build.rs:2-2
Timestamp: 2025-06-28T16:31:03.991Z
Learning: In Cargo build scripts (build.rs), the environment variable CARGO_CFG_TARGET_OS is guaranteed to exist and is automatically set by Cargo during the build process, making unwrap() safe to use when accessing this variable.

Applied to files:

  • example_projects/wasm32_without_js/.gitignore
🧬 Code graph analysis (2)
example_projects/wasm32_without_js/wasm-runtime/src/main.rs (1)
example_projects/wasm32_without_js/rustpython-without-js/src/lib.rs (3)
  • kv_get (3-4)
  • kv_put (7-7)
  • print (10-10)
example_projects/wasm32_without_js/rustpython-without-js/src/lib.rs (1)
example_projects/wasm32_without_js/wasm-runtime/src/main.rs (3)
  • kv_get (13-41)
  • kv_put (44-54)
  • print (75-85)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
  • GitHub Check: Run snippets and cpython tests (windows-latest)
  • GitHub Check: Run snippets and cpython tests on wasm-wasi
  • GitHub Check: Check Rust code with clippy
  • GitHub Check: Check the WASM package and demo
  • GitHub Check: Run snippets and cpython tests (macos-latest)
  • GitHub Check: Run snippets and cpython tests (ubuntu-latest)
  • GitHub Check: Run tests under miri
  • GitHub Check: Run rust tests (windows-latest)
  • GitHub Check: Run rust tests (ubuntu-latest)
  • GitHub Check: Ensure compilation on various targets
  • GitHub Check: Run rust tests (macos-latest)
  • GitHub Check: auto_format
🔇 Additional comments (6)
example_projects/wasm32_without_js/.gitignore (1)

1-2: Standard Rust workspace ignore patterns look appropriate.

The patterns correctly ignore build artifacts (target/) and lock files (Cargo.lock) at the crate level within the workspace, which aligns with typical Rust project conventions for example workspaces.

Optional verification: Confirm that the workspace structure matches the pattern depth (one level of subdirectories). If the workspace root itself generates target/ or Cargo.lock artifacts that need ignoring, add corresponding root-level patterns (target/ and Cargo.lock without the */ prefix). If those are handled by a parent .gitignore in the RustPython repository root, no further action is needed.

example_projects/wasm32_without_js/rustpython-without-js/Cargo.toml (1)

1-4: Verify that using edition = "2024" matches your toolchain and workspace

This crate is set to Rust 2024 edition. Please confirm CI and local toolchains actually support this edition and that it’s consistent with the rest of the workspace; otherwise you may want to stick to the edition used by the main crates (commonly 2021 today) to avoid surprise compilation issues.

example_projects/wasm32_without_js/rustpython-without-js/src/lib.rs (1)

51-60: __getrandom_v03_custom has attribute and behavioral issues

The good news: the current extern "Rust" ABI is correct per getrandom 0.3 documentation.

The remaining issues:

  • #[unsafe(no_mangle)] is invalid syntax; should be #[no_mangle].
  • Returns Ok(()) without writing to dest, leaving the buffer uninitialized (UB).

Fix:

-#[unsafe(no_mangle)]
-unsafe extern "Rust" fn __getrandom_v03_custom(
+#[no_mangle]
+unsafe extern "Rust" fn __getrandom_v03_custom(
     _dest: *mut u8,
     _len: usize,
 ) -> Result<(), getrandom::Error> {
-    // Err(getrandom::Error::UNSUPPORTED)
-
-    // WARNING: This function **MUST** perform proper getrandom
-    Ok(())
+    // TODO: implement proper randomness for wasm32
+    Err(getrandom::Error::UNSUPPORTED)
 }

Likely an incorrect or invalid review comment.

.github/workflows/ci.yaml (1)

412-412: Verify build profile mismatch is intentional.

The wasm-runtime is built with --release but loads the wasm artifact from the debug/ directory. This mismatch may be intentional for testing, but please confirm this is the desired behavior.

example_projects/wasm32_without_js/wasm-runtime/src/main.rs (1)

1-9: LGTM!

The structure and imports are well-organized. The Ctx struct appropriately uses Option<Memory> since the memory reference is established after WASM module instantiation.

example_projects/wasm32_without_js/wasm-runtime/Cargo.toml (1)

7-7: wasmer 6.1.0 is current and secure.

Verification confirms that version 6.1.0 is the latest available version on crates.io. The only known security vulnerability (symlink sandbox bypass) affects versions <= 4.3.1 and does not impact 6.1.0. No action required.

Comment on lines 406 to 412
run: |
cd wasm/wasm-unknown-test
cargo build --release --verbose
if wasm-objdump -xj Import target/wasm32-unknown-unknown/release/wasm_unknown_test.wasm; then
cd example_projects/wasm32_without_js
cargo build --manifest-path rustpython-without-js/Cargo.toml
if wasm-objdump -xj Import rustpython-without-js/target/wasm32-unknown-unknown/debug/rustpython_without_js.wasm; then
echo "ERROR: wasm32-unknown module expects imports from the host environment" >2
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix shell redirect syntax error.

Line 410 uses >2 which will create a file named "2" instead of redirecting to stderr. The correct syntax is >&2.

Apply this diff:

           if wasm-objdump -xj Import rustpython-without-js/target/wasm32-unknown-unknown/debug/rustpython_without_js.wasm; then
-            echo "ERROR: wasm32-unknown module expects imports from the host environment" >2
+            echo "ERROR: wasm32-unknown module expects imports from the host environment" >&2
           fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
run: |
cd wasm/wasm-unknown-test
cargo build --release --verbose
if wasm-objdump -xj Import target/wasm32-unknown-unknown/release/wasm_unknown_test.wasm; then
cd example_projects/wasm32_without_js
cargo build --manifest-path rustpython-without-js/Cargo.toml
if wasm-objdump -xj Import rustpython-without-js/target/wasm32-unknown-unknown/debug/rustpython_without_js.wasm; then
echo "ERROR: wasm32-unknown module expects imports from the host environment" >2
fi
run: |
cd example_projects/wasm32_without_js
cargo build --manifest-path rustpython-without-js/Cargo.toml
if wasm-objdump -xj Import rustpython-without-js/target/wasm32-unknown-unknown/debug/rustpython_without_js.wasm; then
echo "ERROR: wasm32-unknown module expects imports from the host environment" >&2
fi
🤖 Prompt for AI Agents
.github/workflows/ci.yaml around lines 406 to 411: the shell redirection uses
`>2` which creates a file named "2" instead of redirecting to stderr; change the
redirection to `>&2` so the error message is written to stderr (i.e., replace
the `echo "ERROR: ... " >2` with `echo "ERROR: ... " >&2`).

Comment on lines 3 to 17
To test, build rustpython to wasm32-unknown-unknown target first.

```shell
cargo build --manifest-path rustpython-without-js/Cargo.toml
```

Then there will be `rustpython-without-js/target/wasm32-unknown-unknown/debug/rustpython_without_js.wasm` file.

Now we can run the wasm file with wasm runtime:

```shell
cargo run --release --manifest-path wasm-runtime/Cargo.toml rustpython-without-js/target/wasm32-unknown-unknown/debug/rustpython_without_js.wasm
```
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add the explicit wasm32 target to the build command

The text says “build … to wasm32-unknown-unknown target” and then references target/wasm32-unknown-unknown/..., but the sample command doesn’t pass a --target:

cargo build --manifest-path rustpython-without-js/Cargo.toml

To match the described output path, this should be:

-cargo build --manifest-path rustpython-without-js/Cargo.toml
+cargo build --target wasm32-unknown-unknown --manifest-path rustpython-without-js/Cargo.toml

Otherwise users on a default host toolchain will not get the .wasm artifact at the documented location.

🤖 Prompt for AI Agents
In example_projects/wasm32_without_js/README.md around lines 3 to 15, the build
command omits the explicit wasm32 target so the output path shown will not be
produced on a default host toolchain; update the documented cargo build command
to include --target wasm32-unknown-unknown so the .wasm artifact appears under
target/wasm32-unknown-unknown/... (also consider adding the same --target to any
run command examples that reference that path).

Comment on lines 13 to 48
#[unsafe(no_mangle)]
pub unsafe extern "C" fn eval(s: *const u8, l: usize) -> i32 {
// let src = unsafe { std::slice::from_raw_parts(s, l) };
// let src = std::str::from_utf8(src).unwrap();
// TODO: use src
let src = "1 + 3";

// 2. Execute Python code
let interpreter = Interpreter::without_stdlib(Default::default());
let result = interpreter.enter(|vm| {
let scope = vm.new_scope_with_builtins();
let res = match vm.run_block_expr(scope, src) {
Ok(val) => val,
Err(_) => return Err(-1), // Python execution error
};
let repr_str = match res.repr(vm) {
Ok(repr) => repr.as_str().to_string(),
Err(_) => return Err(-1), // Failed to get string representation
};
Ok(repr_str)
});
let result = match result {
Ok(r) => r,
Err(code) => return code,
};

let msg = format!("eval result: {result}");

unsafe {
print(
result.as_str().as_ptr() as usize as i32,
result.len() as i32,
)
};

0
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix no_mangle attribute, use the eval input, and print the full message

There are a few issues in this block:

  • #[unsafe(no_mangle)] is not a valid attribute and will fail to compile.
  • eval ignores its s/l inputs and always runs "1 + 3", which doesn’t match the documented eval(code_ptr, code_len) contract.
  • You build msg = "eval result: {result}" but pass result (without the prefix) to print.

A minimal fix that keeps the current structure but makes it usable would look like:

-#[unsafe(no_mangle)]
-pub unsafe extern "C" fn eval(s: *const u8, l: usize) -> i32 {
-    // let src = unsafe { std::slice::from_raw_parts(s, l) };
-    // let src = std::str::from_utf8(src).unwrap();
-    // TODO: use src
-    let src = "1 + 3";
+#[no_mangle]
+pub unsafe extern "C" fn eval(s: *const u8, l: usize) -> i32 {
+    let src_bytes = std::slice::from_raw_parts(s, l);
+    let src = match std::str::from_utf8(src_bytes) {
+        Ok(s) => s,
+        Err(_) => return -1, // invalid UTF‑8 input
+    };

@@
-    let msg = format!("eval result: {result}");
-
-    unsafe {
-        print(
-            result.as_str().as_ptr() as usize as i32,
-            result.len() as i32,
-        )
-    };
+    let msg = format!("eval result: {result}");
+
+    unsafe {
+        print(
+            msg.as_ptr() as usize as i32,
+            msg.len() as i32,
+        )
+    };

This keeps the ABI, actually uses the caller‑provided code, and prints the prefixed message, while preserving your simple i32 error convention.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#[unsafe(no_mangle)]
pub unsafe extern "C" fn eval(s: *const u8, l: usize) -> i32 {
// let src = unsafe { std::slice::from_raw_parts(s, l) };
// let src = std::str::from_utf8(src).unwrap();
// TODO: use src
let src = "1 + 3";
// 2. Execute Python code
let interpreter = Interpreter::without_stdlib(Default::default());
let result = interpreter.enter(|vm| {
let scope = vm.new_scope_with_builtins();
let res = match vm.run_block_expr(scope, src) {
Ok(val) => val,
Err(_) => return Err(-1), // Python execution error
};
let repr_str = match res.repr(vm) {
Ok(repr) => repr.as_str().to_string(),
Err(_) => return Err(-1), // Failed to get string representation
};
Ok(repr_str)
});
let result = match result {
Ok(r) => r,
Err(code) => return code,
};
let msg = format!("eval result: {result}");
unsafe {
print(
result.as_str().as_ptr() as usize as i32,
result.len() as i32,
)
};
0
}
#[no_mangle]
pub unsafe extern "C" fn eval(s: *const u8, l: usize) -> i32 {
let src_bytes = std::slice::from_raw_parts(s, l);
let src = match std::str::from_utf8(src_bytes) {
Ok(s) => s,
Err(_) => return -1, // invalid UTF‑8 input
};
// 2. Execute Python code
let interpreter = Interpreter::without_stdlib(Default::default());
let result = interpreter.enter(|vm| {
let scope = vm.new_scope_with_builtins();
let res = match vm.run_block_expr(scope, src) {
Ok(val) => val,
Err(_) => return Err(-1), // Python execution error
};
let repr_str = match res.repr(vm) {
Ok(repr) => repr.as_str().to_string(),
Err(_) => return Err(-1), // Failed to get string representation
};
Ok(repr_str)
});
let result = match result {
Ok(r) => r,
Err(code) => return code,
};
let msg = format!("eval result: {result}");
unsafe {
print(
msg.as_ptr() as usize as i32,
msg.len() as i32,
)
};
0
}
🤖 Prompt for AI Agents
In example_projects/wasm32_without_js/rustpython-without-js/src/lib.rs around
lines 13 to 49, fix three issues: change the invalid attribute to #[no_mangle]
(keep the function as pub unsafe extern "C" fn eval(...)), actually read the
caller-provided code by creating a slice from_raw_parts(s, l) and converting it
to &str (return an error code on null/utf8 failure), and build the full message
using the formatted prefix (e.g. "eval result: {repr}") then pass that message's
pointer and length to print (cast pointer/len to the i32 ABI types you use) so
the printed output contains the prefix and the evaluated result.

[package]
name = "wasm-runtime"
version = "0.1.0"
edition = "2024"
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix invalid Rust edition.

Edition 2024 does not exist. The latest stable Rust edition is 2021. This will cause a compilation failure.

Apply this diff to fix:

-edition = "2024"
+edition = "2021"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
edition = "2024"
edition = "2021"
🤖 Prompt for AI Agents
In example_projects/wasm32_without_js/wasm-runtime/Cargo.toml around line 4, the
Cargo.toml incorrectly sets edition = "2024"; change it to the latest valid Rust
edition by replacing the value with "2021" so the crate compiles successfully.

Comment on lines +13 to +41
fn kv_get(mut ctx: FunctionEnvMut<Ctx>, kp: i32, kl: i32, vp: i32, vl: i32) -> i32 {
let (c, s) = ctx.data_and_store_mut();
let mut key = vec![0u8; kl as usize];
if c.mem
.as_ref()
.unwrap()
.view(&s)
.read(kp as u64, &mut key)
.is_err()
{
return -1;
}
match c.kv.get(&key) {
Some(val) => {
let len = val.len().min(vl as usize);
if c.mem
.as_ref()
.unwrap()
.view(&s)
.write(vp as u64, &val[..len])
.is_err()
{
return -1;
}
len as i32
}
None => 0,
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Replace unwrap() with proper error handling.

Lines 18 and 30 call unwrap() on c.mem.as_ref(), which will panic if the memory reference is None. While this should never happen if the module is correctly initialized, it creates a potential panic point.

Consider using expect() with a descriptive message or returning an error code:

 fn kv_get(mut ctx: FunctionEnvMut<Ctx>, kp: i32, kl: i32, vp: i32, vl: i32) -> i32 {
     let (c, s) = ctx.data_and_store_mut();
     let mut key = vec![0u8; kl as usize];
-    if c.mem
-        .as_ref()
-        .unwrap()
-        .view(&s)
+    let mem = c.mem.as_ref().expect("memory not initialized");
+    if mem.view(&s)
         .read(kp as u64, &mut key)
         .is_err()
     {
         return -1;
     }
     match c.kv.get(&key) {
         Some(val) => {
             let len = val.len().min(vl as usize);
-            if c.mem
-                .as_ref()
-                .unwrap()
-                .view(&s)
+            if mem.view(&s)
                 .write(vp as u64, &val[..len])
                 .is_err()
             {
                 return -1;
             }
             len as i32
         }
         None => 0,
     }
 }
🤖 Prompt for AI Agents
In example_projects/wasm32_without_js/wasm-runtime/src/main.rs around lines 13
to 41, replace the two uses of c.mem.as_ref().unwrap() with safe error handling:
check whether c.mem.as_ref() is Some and if not return an appropriate error code
(e.g. -1) or use expect() with a clear message if you prefer a hard failure;
then use the unwrapped reference afterward (or bind it to a variable) so the
read/write calls operate on a guaranteed memory reference and avoid potential
panics.

Comment on lines +44 to +54
fn kv_put(mut ctx: FunctionEnvMut<Ctx>, kp: i32, kl: i32, vp: i32, vl: i32) -> i32 {
let (c, s) = ctx.data_and_store_mut();
let mut key = vec![0u8; kl as usize];
let mut val = vec![0u8; vl as usize];
let m = c.mem.as_ref().unwrap().view(&s);
if m.read(kp as u64, &mut key).is_err() || m.read(vp as u64, &mut val).is_err() {
return -1;
}
c.kv.insert(key, val);
0
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Replace unwrap() with proper error handling.

Line 48 calls unwrap() on c.mem.as_ref(), which creates a potential panic point if memory is not initialized.

Apply similar fix as suggested for kv_get:

 fn kv_put(mut ctx: FunctionEnvMut<Ctx>, kp: i32, kl: i32, vp: i32, vl: i32) -> i32 {
     let (c, s) = ctx.data_and_store_mut();
     let mut key = vec![0u8; kl as usize];
     let mut val = vec![0u8; vl as usize];
-    let m = c.mem.as_ref().unwrap().view(&s);
+    let m = c.mem.as_ref().expect("memory not initialized").view(&s);
     if m.read(kp as u64, &mut key).is_err() || m.read(vp as u64, &mut val).is_err() {
         return -1;
     }
     c.kv.insert(key, val);
     0
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fn kv_put(mut ctx: FunctionEnvMut<Ctx>, kp: i32, kl: i32, vp: i32, vl: i32) -> i32 {
let (c, s) = ctx.data_and_store_mut();
let mut key = vec![0u8; kl as usize];
let mut val = vec![0u8; vl as usize];
let m = c.mem.as_ref().unwrap().view(&s);
if m.read(kp as u64, &mut key).is_err() || m.read(vp as u64, &mut val).is_err() {
return -1;
}
c.kv.insert(key, val);
0
}
fn kv_put(mut ctx: FunctionEnvMut<Ctx>, kp: i32, kl: i32, vp: i32, vl: i32) -> i32 {
let (c, s) = ctx.data_and_store_mut();
let mut key = vec![0u8; kl as usize];
let mut val = vec![0u8; vl as usize];
let m = c.mem.as_ref().expect("memory not initialized").view(&s);
if m.read(kp as u64, &mut key).is_err() || m.read(vp as u64, &mut val).is_err() {
return -1;
}
c.kv.insert(key, val);
0
}
🤖 Prompt for AI Agents
In example_projects/wasm32_without_js/wasm-runtime/src/main.rs around lines 44
to 54, replace the direct unwrap() on c.mem.as_ref() with safe handling: check
whether c.mem is Some and if not return an error code (e.g., -1) like kv_get
does; if Some(mem) obtain the memory view (let m = mem.view(&s)) and continue
with the existing reads and inserts, preserving the current read error checks
and returning -1 on failure and 0 on success.

Comment on lines +75 to +85
fn print(mut ctx: FunctionEnvMut<Ctx>, p: i32, l: i32) -> i32 {
let (c, s) = ctx.data_and_store_mut();
let mut msg = vec![0u8; l as usize];
let m = c.mem.as_ref().unwrap().view(&s);
if m.read(p as u64, &mut msg).is_err() {
return -1;
}
let s = std::str::from_utf8(&msg).expect("print got non-utf8 str");
println!("{s}");
0
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Improve error handling for memory access and UTF-8 validation.

Two issues:

  1. Line 78: unwrap() on c.mem.as_ref() creates a panic point
  2. Line 82: expect() will panic if the WASM module passes non-UTF8 data

Consider this more robust approach:

 fn print(mut ctx: FunctionEnvMut<Ctx>, p: i32, l: i32) -> i32 {
     let (c, s) = ctx.data_and_store_mut();
     let mut msg = vec![0u8; l as usize];
-    let m = c.mem.as_ref().unwrap().view(&s);
+    let m = c.mem.as_ref().expect("memory not initialized").view(&s);
     if m.read(p as u64, &mut msg).is_err() {
         return -1;
     }
-    let s = std::str::from_utf8(&msg).expect("print got non-utf8 str");
-    println!("{s}");
-    0
+    match std::str::from_utf8(&msg) {
+        Ok(s) => {
+            println!("{s}");
+            0
+        }
+        Err(_) => {
+            eprintln!("print received non-UTF8 data");
+            -1
+        }
+    }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fn print(mut ctx: FunctionEnvMut<Ctx>, p: i32, l: i32) -> i32 {
let (c, s) = ctx.data_and_store_mut();
let mut msg = vec![0u8; l as usize];
let m = c.mem.as_ref().unwrap().view(&s);
if m.read(p as u64, &mut msg).is_err() {
return -1;
}
let s = std::str::from_utf8(&msg).expect("print got non-utf8 str");
println!("{s}");
0
}
fn print(mut ctx: FunctionEnvMut<Ctx>, p: i32, l: i32) -> i32 {
let (c, s) = ctx.data_and_store_mut();
let mut msg = vec![0u8; l as usize];
let m = c.mem.as_ref().expect("memory not initialized").view(&s);
if m.read(p as u64, &mut msg).is_err() {
return -1;
}
match std::str::from_utf8(&msg) {
Ok(s) => {
println!("{s}");
0
}
Err(_) => {
eprintln!("print received non-UTF8 data");
-1
}
}
}

Comment on lines +87 to +93
fn main() {
let mut store = Store::default();
let module = Module::new(
&store,
&std::fs::read(&std::env::args().nth(1).unwrap()).unwrap(),
)
.unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add command-line argument validation.

Line 91 calls nth(1).unwrap() without checking if the argument exists, which will panic if no WASM file path is provided. Consider adding proper error handling.

Apply this diff:

 fn main() {
+    let wasm_path = std::env::args()
+        .nth(1)
+        .expect("Usage: wasm-runtime <wasm_file_path>");
+    
     let mut store = Store::default();
-    let module = Module::new(
-        &store,
-        &std::fs::read(&std::env::args().nth(1).unwrap()).unwrap(),
-    )
-    .unwrap();
+    let wasm_bytes = std::fs::read(&wasm_path)
+        .expect("Failed to read WASM file");
+    let module = Module::new(&store, &wasm_bytes)
+        .expect("Failed to create WASM module");
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fn main() {
let mut store = Store::default();
let module = Module::new(
&store,
&std::fs::read(&std::env::args().nth(1).unwrap()).unwrap(),
)
.unwrap();
fn main() {
let wasm_path = std::env::args()
.nth(1)
.expect("Usage: wasm-runtime <wasm_file_path>");
let mut store = Store::default();
let wasm_bytes = std::fs::read(&wasm_path)
.expect("Failed to read WASM file");
let module = Module::new(&store, &wasm_bytes)
.expect("Failed to create WASM module");
🤖 Prompt for AI Agents
In example_projects/wasm32_without_js/wasm-runtime/src/main.rs around lines 87
to 93, the code calls std::env::args().nth(1).unwrap() which will panic if no
argument is provided; update the main function to validate the presence of the
first command-line argument, return a clear error or print a usage message and
exit when it is missing, and then use the validated path when reading the file
(e.g., check args().nth(1).ok_or_else(|| ...) or pattern-match and handle the
None case before calling std::fs::read and Module::new).

Comment on lines 117 to 125
let inst = Instance::new(&mut store, &module, &imports).unwrap();
env.as_mut(&mut store).mem = inst.exports.get_memory("memory").ok().cloned();
let res = inst
.exports
.get_function("eval")
.unwrap()
.call(&mut store, &[wasmer::Value::I32(0), wasmer::Value::I32(0)])
.unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Replace unwrap() calls and document eval arguments.

Multiple issues:

  1. Lines 117, 122, 124: unwrap() calls that could panic
  2. Line 123: The eval function is called with (0, 0) arguments - their meaning is undocumented

Apply this diff for better error handling:

-    let inst = Instance::new(&mut store, &module, &imports).unwrap();
+    let inst = Instance::new(&mut store, &module, &imports)
+        .expect("Failed to instantiate WASM module");
     env.as_mut(&mut store).mem = inst.exports.get_memory("memory").ok().cloned();
-    let res = inst
-        .exports
-        .get_function("eval")
-        .unwrap()
-        .call(&mut store, &[wasmer::Value::I32(0), wasmer::Value::I32(0)])
-        .unwrap();
+    let eval_fn = inst
+        .exports
+        .get_function("eval")
+        .expect("WASM module does not export 'eval' function");
+    // TODO: Document what these arguments (0, 0) represent
+    let res = eval_fn
+        .call(&mut store, &[wasmer::Value::I32(0), wasmer::Value::I32(0)])
+        .expect("Failed to call eval function");
🤖 Prompt for AI Agents
In example_projects/wasm32_without_js/wasm-runtime/src/main.rs around lines 117
to 124, replace the three unwrap() calls with proper error handling and document
the eval function arguments: check and return or propagate errors when creating
the Instance, when getting the exported memory, and when locating/calling the
"eval" function (e.g., use match/if let or ? with anyhow::Context to add
descriptive messages like "failed to instantiate module", "memory export not
found", "eval function not found" and include the underlying error), and add a
brief comment above the call explaining what the two i32 arguments represent
(for example, pointer/length or args/indexes) and derive or compute the correct
argument values instead of hardcoding (0,0) or validate that (0,0) is intended
before calling.

youknowone and others added 2 commits November 17, 2025 21:30
Co-Authored-By: Valentyn Faychuk <valy@faychuk.com>
Co-Authored-By: Lee Dogeon <dev.moreal@gmail.com>
@youknowone youknowone force-pushed the test-wasm32-without-js branch from 1a92d4f to 9134cca Compare November 17, 2025 12:30
@youknowone youknowone merged commit bab03bd into RustPython:main Nov 17, 2025
12 checks passed
@youknowone youknowone deleted the test-wasm32-without-js branch November 17, 2025 13:10
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.

wasm32 build without js doesn't have example and test

1 participant