Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Moving def of EQUS to below usage makes value equal to its length #751

Closed
dannye opened this issue Feb 21, 2021 · 2 comments · Fixed by #752
Closed

Moving def of EQUS to below usage makes value equal to its length #751

dannye opened this issue Feb 21, 2021 · 2 comments · Fixed by #752
Labels
bug Unexpected behavior / crashes; to be fixed ASAP! rgbasm This affects RGBASM

Comments

@dannye
Copy link
Contributor

dannye commented Feb 21, 2021

Here is a boiled down example of a situation I ran into today.
Given this file, bug.asm:

SECTION "Bug", ROM0

PART_TWO_START EQUS "Table.part_two - Table.part_one"

PartTwoIDs:
    db PART_TWO_START + 0 ; $04
    db PART_TWO_START + 1 ; $05
    db PART_TWO_START + 2 ; $06
    db PART_TWO_START + 3 ; $07

Table:
.part_one
    db $00
    db $10
    db $20
    db $30
.part_two
    db $40
    db $50
    db $60
    db $70

Using rgbds v0.4.2, if you run rgbasm bug.asm -o bug.o && rgblink bug.o -o bug.gbc, then the first 12 bytes of bug.gbc will be as expected:

04 05 06 07 00 10 20 30 40 50 60 70

For sanity, I tried moving the definition of PART_TWO_START down after where it is used to see what the error message would be:

SECTION "Bug", ROM0

PartTwoIDs:
    db PART_TWO_START + 0 ; $04
    db PART_TWO_START + 1 ; $05
    db PART_TWO_START + 2 ; $06
    db PART_TWO_START + 3 ; $07

PART_TWO_START EQUS "Table.part_two - Table.part_one"

Table:
.part_one
    db $00
    db $10
    db $20
    db $30
.part_two
    db $40
    db $50
    db $60
    db $70

However, the ROM still builds fine, and the first 12 bytes of bug.gbc are:

1F 20 21 22 00 10 20 30 40 50 60 70

Here, 0x1f is the length of the string "Table.part_two - Table.part_one".

@ISSOtm ISSOtm added bug Unexpected behavior / crashes; to be fixed ASAP! rgbasm This affects RGBASM labels Feb 21, 2021
@ISSOtm
Copy link
Member

ISSOtm commented Feb 21, 2021

This is indeed a bug, but this should error out at the definition of PART_TWO_START.

EQUS expansion only occurs if the EQUS is defined prior. This is because the expansion has to take place as the code is read. So the PART_TWO_STARTs in PartTwoIDs are not EQUS expansions, but are treated as symbol references (i.e. numeric values whose evaluation is deferred to RGBLINK).

Since it's marked as "referenced", the symbol ends up being emitted in the object file, which outputs the value field... except that the symbol is an EQUS, so we instead deref the "value union" with the wrong member, which happens to pick (on your machine) the macroSize field: the string's length!

As for numeric symbols, the reason why you've been unable to use PART_TWO_START equ Table.part_two - Table.part_one is simply that numeric symbols must have a value known to RGBASM, they can't store arbitrary expressions. (This is what #201 would be for.)

All this lengthy explanation boils down to these points:

  • EQUS need to be known before they're supposed to be expanded
  • "References" should not be overridden by EQUS or other non-numeric symbols
  • Numeric symbols need to have "constant" values

So, while we certainly have a bug (sym_AddString and sym_AddMacro should ensure that the symbol doesn't exist), the second version of the example code is invalid anyway. Here are 3 possible alternatives:

  • Use the first version; this will work, but you said you didn't like it
  • Spell out the full expression each time, giving RGBASM a bit less work, but losing semantics & increasing redundancy
  • Put PART_TWO_START equ Table.part_two - Table.part_one after the labels

Here's why the last one will work:

  1. After all the labels have been defined, their values are not known to RGBASM (unless the section they're in has a fixed address)
  2. However, since they both belong to the same section, RGBASM is able to compute their difference (see the last paragraph of "Label definition" in the language documentation)
  3. Therefore, this expression is constant only after both symbols has been defined; but since it is, it can be assigned to a numeric symbol
  4. The numeric symbol is defined, and is referenced earlier in the file, so it ends up being emitted, and the dbs will be computed & patched in by RGBLINK

This solution allows defining PART_TWO_START after everything else; but there is (currently) no way to define it where you seem to want. Further, it also gives a bit more work to RGBASM, and also to RGBLINK (since RGBASM does not compute the value, and everything has to go through the object file).

There is no way to define it "in the middle": as soon as a symbol is referenced, it can only be defined as a numeric symbol (label, equ, set/=). Even a function would need to be defined before being referenced, at least the way they're currently planned.

@dannye
Copy link
Contributor Author

dannye commented Feb 21, 2021

Cool explanation! Thanks.

Use the first version; this will work, but you said you didn't like it

Oh, no, I have no issue with the first approach. I only tried the second approach because I was goofing off.
Although, I may use your third suggestion so that I can switch to EQU. Either way, the bug is not inconveniencing me, I just wanted to share.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected behavior / crashes; to be fixed ASAP! rgbasm This affects RGBASM
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants