Change object file format to not reuse "patch" mechanism for assertions
- Dominant language
- C++
- Stars
- 1.6k
- Forks
- 193
- Avg merge
- 22h 17m
- Merged PRs (30d)
- 26
Description
In both RGBASM and RGBLINK, the `struct Section` stores a collection of `struct Patch`es, which represent bytes that get patched over with values computed at link time from RPN expressions.
When #488 implemented assertions, it reused code from the patch mechanism to store RPN expressions which were being asserted to be true (nonzero). This had two consequences I'd like to fix.
One is that the `Offset` field of a patch is unused by the assertion (documented in `rgbds(5)` as an "unused leftover from the patch structure"), so it just bloats the object files a little bit.
The other is that the `Type` field of a patch ends up having two different meanings. For section patches it can be 0, 1, 2, or 3, indicating the type of patch (byte, word, long, or `jr`). For assertion patches it can be 0, 1, or 2, indicating the type of assertion (warning, error, or fatal). This results in some suspicious-looking code casting to/from contradictory `enum` values.
I'd rather just have separate code to write→read sections and assertions, without trying to reuse `writePatch` and `readPatch` functions for both. If we want to avoid duplicate code, there can be smaller reused units like "write/read `RPNSize` followed by `RPNExpr`". This would be a breaking change if we remove the unused `Offset` field from assertions in the object file.
Contributor guide
Research direction
Start by reading the struct Section definitions in RGBASM and RGBLINK, then trace the writePatch and readPatch paths for sections and assertions. The work is complete when assertions and section patches use distinct object-file serialization paths, with only smaller RPNSize/RPNExpr units shared where appropriate and the unused assertion Offset removed.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- compilers
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100