WASM parsing with complex nested structs - #387
Conversation
|
@quickweb-stack Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughWalkthroughChangesDecoder and contract-spec updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant wasm_tests
participant decode_contract_spec
participant ContractSpec
wasm_tests->>decode_contract_spec: provide synthesized contractspecv0 WASM payload
decode_contract_spec->>ContractSpec: decode nested ScSpecEntry definitions
ContractSpec-->>wasm_tests: return decoded structs and type definitions
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Done, Close: #387 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/core/src/spec/decoder.rs (1)
538-546: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winDuplicate
enums/unionsfields in struct literal — will not compile.
enums: Vec::new()andunions: Vec::new()are already set at Lines 540-541; the newly added Lines 544-545 set them again in the sameContractSpec { ... }literal. Rust rejects duplicate field initializers in a struct literal (E0062), so this fixture will fail to compile, contradicting the PR's claim that all spec tests pass.🐛 Proposed fix
functions: Vec::new(), structs: Vec::new(), enums: Vec::new(), unions: Vec::new(), name: None, version: None, - enums: Vec::new(), - unions: Vec::new(), };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/core/src/spec/decoder.rs` around lines 538 - 546, Remove the duplicate enums and unions initializers from the ContractSpec struct literal in the fixture, leaving each field initialized exactly once alongside functions and structs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/core/src/spec/decoder.rs`:
- Around line 538-546: Remove the duplicate enums and unions initializers from
the ContractSpec struct literal in the fixture, leaving each field initialized
exactly once alongside functions and structs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 05f7c464-a370-4171-a9f9-409324cfd15a
📒 Files selected for processing (7)
crates/core/src/decode/function_call_decoder.rscrates/core/src/decode/mod.rscrates/core/src/decode/return_decoder.rscrates/core/src/spec/decoder.rscrates/core/src/spec/mod.rscrates/core/src/spec/tests/mod.rscrates/core/src/spec/tests/wasm_tests.rs
💤 Files with no reviewable changes (1)
- crates/core/src/decode/mod.rs
Close: #374
Here's a summary of what was done:
Changes Made
New files
Modified files
Tests added
All 9 spec tests pass (5 new + 4 existing).
Summary by CodeRabbit
New Features
Bug Fixes
Tests