gbdev / gbdev/rgbds

Change object file format to not reuse "patch" mechanism for assertions

Open
#2,019 0 comments 0 reactions 0 assignees View on GitHub
breaking optimization refactoring rgbasm rgblink
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.