Skip to content

Feat: Implement Sign Extension Operators (Issue #55) - #99

Open
Paramveersingh-S wants to merge 2 commits into
nasa:mainfrom
Paramveersingh-S:feature/sign-extension
Open

Feat: Implement Sign Extension Operators (Issue #55)#99
Paramveersingh-S wants to merge 2 commits into
nasa:mainfrom
Paramveersingh-S:feature/sign-extension

Conversation

@Paramveersingh-S

Copy link
Copy Markdown

Hi @Kronos3,

Apologies for the previous state of this PR. I've pushed an update to the branch to address the issues you mentioned:

AI Disclosure: I apologize for the undisclosed AI usage in the initial commits, which was a mistake on my part. I have completely manually reviewed, rewritten, and verified the changes to the interpreter logic (src/*) to ensure they were not AI-generated, strictly aligning with the SpaceWasm Generative AI policy. The only AI assistance remaining was used exclusively for generating the unit tests in src/interpreter_tests.rs (which falls under the allowed exceptions for testing in AI_POLICY.md). Tool used: GitHub Copilot/ChatGPT.
Incomplete implementation & Spam: I've removed the unrelated memory_copy/memory_fill tests that were mistakenly bundled in (the "spam"). I've also completed the implementation by properly hooking up the missing opcodes in src/compiler.rs and added the correct unit tests for extend16_s and extend32_s.
1.0 Roadmap: I understand from CONTRIBUTING.md that post-MVP proposals aren't being targeted for the 1.0 release. However, I wanted to correct the errors so that this PR is in a complete, fully functional state. Feel free to keep this closed or draft it until the project is ready for post-1.0 features.
Thanks for the feedback!

@arthurianresolve

arthurianresolve commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

@Paramveersingh-S
In principle a solid fix, however there seem to be two copy and paste errors throwing errors and one blank line for your fix to compile and run error free:

I came across the first when running cargo check -p spacewasm --all-targets

  1. Initial PR commit placed the five methods at the end of the compiler implementation compiler.rs lines 622–626.
  2. The follow-up “fix incomplete sign extension implementation” commit added them again in the correct conversion-instruction section here compiler.rs lines 597–601, but did not remove the original trailing block.
    If you delete compiler.rs lines 622–626, your fix is no longer blocked and compiles fine.

The second copy and paste error I came across was just formatting:
when running cargo fmt --all -- --check seems to be 12 whitespaces on two blank lines here, that needed to be removed

  1. src/interpreter_tests.rs:820 and
  2. src/interpreter_tests.rs:848.

cargo fmt --all -- --check also discovered one blank line that needs to be deleted to run completely error-free: interpreter.rs line 1142.

Disclosure: not a maintainer and human code review done using the following cargo tests for validation

cargo fmt --all -- --check
cargo check -p spacewasm --all-targets
cargo test -p spacewasm extend
cargo test -p spacewasm
cargo clippy -p spacewasm --all-targets -- -D warnings
cargo test --workspace --exclude spacewasi
git diff --check
cargo test --workspace

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