Closed StanHash closed 5 years ago
Stan you always make quality code, so it's not like I don't trust this, buuuuut Mind quickly outlining the test cases you used to verify this function?
Assignment to known constant (could have been implemented even without changing the label system)
Event:
A = 4
BYTE 1 2 3 A
ROM (Hex):
01 02 03 04
Assignment to unknown (when parsing) constant (requires "saving" the expression for later computation)
Event:
A = hello
BYTE 1 2 3 A
hello:
WORD -1
ROM (Hex):
01 02 03 04 ff ff ff ff
Assignment to "complex" math expression (and usage in ORG
statement)
Event:
A = Table + B * 4
B = 2
ORG $10
Table: WORD 0 0 0 0
Data:
PUSH; ORG A; POIN Data; POP
ROM (Hex):
00000000 | (unchanged)
00000010 | 00 00 00 00 00 00 00 00 20 00 00 08 00 00 00 00
CURRENTOFFSET
relative
Event:
ORG $10
A = B - CURRENTOFFSET // Should be 8
WORD 0
WORD A
B:
ROM (Hex):
00000010 | 00 00 00 00 08 00 00 00
Note: NL!EA 11.1 would end up writing 4, as CURRENTOFFSET
wouldn't get expanded before the WORD A
statement invokes the computation of A
. I consider this an improvement, as symbol values should be constants.
Circular symbol dependency
Event:
A = B
B = C
C = A
WORD A B C
stderr:
Errors:
In File Main.event, Line 3, Column 1: Assigned symbol depends on itself!
Errors occurred; no changes written.
(I also tested with other various combinations of 1 to 3 symbols that depend on themselves; Including some with math involved and stuff)
Uncomputable symbol
Event:
A = B
WORD A
stderr:
Unidentified identifier when evaluating tree: File <...>/Main.event, Line 2, Column 6, IDENTIFIER: A
This one probably could have better diagnosis, as we know the symbol exists; All we don't is its computed value.
Note: uncomputable symbols won't be reported unless observed: just writing A = B
and then never refer to A
at any point (unless in other symbol assignment statements; that are also not referred to) won't result in an error. I understand it's debatable whether this is acceptable behavior or not.
Out of scope symbol
Event:
{ A = 1 }
WORD A
stderr:
Unidentified identifier when evaluating tree: File <...>/Main.event, Line 2, Column 6, IDENTIFIER: A
Same remark as for previous one.
All done with the following commandline:
Core A FE8 -input:<...>/Main.event -output:<...>/ROM.bin
And with the raws that come with the current 11.1.3 release.
Someone teach me how to write proper automated tests
Okay, so I'll need to take a look at how the symbols are stored (should be akin to labels? but distinct since they can be expressions) and double check cycle dependency detection.
Do you think it'd be worth rewriting FE7/8 definitions as symbols instead of defines at some point?
I don't know... Them being #undef-able sounds like something useful. Them being symbols would also mean that they are going to be output as such when/if we implement -symOutput
too; Which I don't think is really the best for those. (And also I just kind of like the idea of the stdlib being strictly a macro library).
I'm not happy with this as is so I'll close until I feel like working on this again.
I'd be interested in implementing a simplified version of this.
Rather than having any complex symbol resolution (lazy evaluation), just make it eager.
A = B + C
tries to evaluate B and C on the spot, and if it can't, it throws an error.
This should be sufficient for most uses (and anything more complex can use #define).
Thanks to ColorzCore's internal improvements, this implementation of the feature shouldn't freak out when assigning symbols to expressions invoking
CURRENTOFFSET
while not being immediately computable (because ofCURRENTOFFSET
now being substituted at parse-time (?)).Another improvement over the 11.1 impl is that circular dependencies are detected and diagnosed. For example, the following:
Will print this error:
Some files seem to have been detected as having completely been rewritten... is this caused by a difference in encoding?
Hopefully I didn't taint your code of my incompetence too much.(but do tell me if I did)