Added getMemory to api-debugger#92
Conversation
Added `getMemory` to API
WalkthroughAdded a debugger memory-read API Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant DebuggerAPI as api-debugger
participant Interpreter
Note over DebuggerAPI,Interpreter #DDEBF7: getMemory flow
Client->>DebuggerAPI: getMemory(address, length)
DebuggerAPI->>Interpreter: check interpreter
alt interpreter null
DebuggerAPI-->>Client: return Uint8Array([])
else
DebuggerAPI->>Interpreter: memory.bytesRead(address, length, MaybePageFault)
alt page fault
DebuggerAPI-->>Client: return Uint8Array([])
else success
DebuggerAPI-->>Client: return filled Uint8Array
end
end
sequenceDiagram
participant Caller as runVm/Caller
participant Executor as executeProgram
participant Memory as int.memory
participant Logger
Note over Executor,Memory #FFF2CC: Execution end behavior change
Caller->>Executor: executeProgram(...)
Executor->>Logger: log outcome
alt outcome != ok
Executor->>Logger: log "Exit code: {int.exitCode}"
end
Caller->>Memory: int.memory.free() -- moved here (in runVm)
Note right of Executor: Previously: Memory.free() called inside executeProgram
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (2)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
assembly/api-internal.ts (1)
178-186: Reintroduce int.memory.free() after extracting memory chunks to avoid leaks
executeProgram no longer frees int.memory, and neither runVm nor its callers free it, leading to a potential memory leak; re-add int.memory.free() after getOutputChunks(int.memory) or ensure cleanup elsewhere.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
assembly/api-debugger.ts(1 hunks)assembly/api-internal.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
assembly/api-debugger.ts (1)
assembly/interpreter.ts (1)
Interpreter(21-200)
🪛 GitHub Actions: Node.js CI
assembly/api-debugger.ts
[error] 176-176: TS2554: Expected 4 arguments, but got 2. (in assembly/api-debugger.ts:176)
[error] 176-176: TS2322: Type 'void' is not assignable to type ''. (in assembly/api-debugger.ts:176)
[error] 177-177: TS2304: Cannot find name 'fault'. (in assembly/api-debugger.ts:177)
🔇 Additional comments (1)
assembly/api-internal.ts (1)
150-150: LGTM! Improved error diagnostics.The addition of exit code logging enhances debugging capabilities by providing more context when execution fails.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
assembly/api-debugger.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
assembly/api-debugger.ts (2)
assembly/interpreter.ts (1)
Interpreter(21-200)assembly/memory.ts (1)
MaybePageFault(15-19)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
README.md (1)
3-3: Use canonical project nameAssemblyScript.Official naming keeps it as one word. Adjusting avoids confusion in docs.
-Assembly Script implementation of the JAM PVM (64bit). +AssemblyScript implementation of the JAM PVM (64bit).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~3-~3: Ensure spelling is correct
Context: ...y Script implementation of the JAM PVM (64bit). Demo ###...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
README.md (2)
95-96: Standardize shell fence language to bashCommands are OS‑agnostic. Using bash aligns with other blocks and supports inline comments if needed.
-```cmd +```bash npm ci-```cmd +```bash npm run build-```cmd +```bash npm run web-```cmd +```bash npm start ./path/to/tests/*.jsonAlso applies to: 101-102, 107-108, 113-114
56-61: Document the new debugger getMemory API (example usage)Add a brief note and example so users can discover it easily from README.
Proposed addition after the Raw Bindings paragraph:
### Debugger API The debugger exposes helpers for inspecting memory. For example, `getMemory(address: u32, length: u32): Uint8Array` returns a slice of linear memory. ESM: ```javascript import ananAs from '@fluffylabs/anan-as/debug'; // ... after running a program const bytes = ananAs.getMemory(0x1000, 64); console.log(bytes);Raw bindings:
import { instantiate } from '@fluffylabs/anan-as/raw'; const { exports } = await instantiate(module); // ... after running a program const bytes = exports.getMemory(0x1000, 64); console.log(bytes);</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between c641288cdf32930efa0f511f28e4fa0b43fab774 and 42a544665952f6fe01ec6cae3900108314d2c1fa. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `README.md` (4 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 LanguageTool</summary> <details> <summary>README.md</summary> [grammar] ~3-~3: Ensure spelling is correct Context: ...y Script implementation of the JAM PVM (64bit). [Demo](https://todr.me/anan-as) ## ... (QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1) --- [grammar] ~9-~9: There might be a mistake here. Context: .../todr.me/anan-as) ## Todo - [x] Memory - [x] [JAM tests](https://github.com/w3f/j... (QB_NEW_EN) --- [grammar] ~10-~10: There might be a mistake here. Context: ...w3f/jamtestvectors/pull/3) compatibility - [x] 64-bit & new instructions ([GrayPape... (QB_NEW_EN) --- [grammar] ~11-~11: There might be a mistake here. Context: ....5.0](https://graypaper.fluffylabs.dev)) - [x] GP 0.5.4 compatibility (ZBB extensio... (QB_NEW_EN) --- [grammar] ~16-~16: There might be a mistake here. Context: .../en.wikipedia.org/wiki/Ananas) are cool. - [JAM](https://graypaper.com/) is promisin... (QB_NEW_EN) --- [grammar] ~80-~80: There might be a mistake here. Context: ..., you can choose between stable releases and bleeding-edge builds: ```bash # Lat... (QB_NEW_EN) --- [style] ~110-~110: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. Context: ...calhost:3000). ```cmd npm run web ``` To run JSON test vectors. ```cmd npm star... (ENGLISH_WORD_REPEAT_BEGINNING_RULE) </details> </details> </details> <details> <summary>🔇 Additional comments (4)</summary><blockquote> <details> <summary>README.md (4)</summary><blockquote> `7-7`: **Heading level change looks good** Promoting “Todo” to H2 improves structure. --- `14-14`: **Heading level change looks good** Consistent H2 hierarchy. --- `20-20`: **Heading level change looks good** Clearer sectioning. --- `78-82`: **Version tags section LGTM** Clear guidance for stable vs. next. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
2 participants
deleted
memory.free()and loggingexit codeSummary by CodeRabbit
New Features
Improvements
Documentation