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

Fix expansions that start from the end of another expansion #839

Merged
merged 1 commit into from
Apr 17, 2021

Conversation

Rangi42
Copy link
Contributor

@Rangi42 Rangi42 commented Apr 17, 2021

Do not free an expansion until its offset is past its size. This means potentially freeing a nested stack of expansions all at once.

Fixes #696

@Rangi42 Rangi42 added bug Unexpected behavior / crashes; to be fixed ASAP! rgbasm This affects RGBASM labels Apr 17, 2021
@Rangi42 Rangi42 added this to the v0.5.0 milestone Apr 17, 2021
@Rangi42 Rangi42 requested a review from ISSOtm April 17, 2021 16:44
@Rangi42 Rangi42 force-pushed the expansion-nesting branch from 0a10197 to 25c41e0 Compare April 17, 2021 16:49
src/asm/lexer.c Outdated Show resolved Hide resolved
src/asm/lexer.c Outdated Show resolved Hide resolved
@Rangi42 Rangi42 force-pushed the expansion-nesting branch 2 times, most recently from 3d3e7ca to f8c4a80 Compare April 17, 2021 17:00
Do not free an expansion until its offset is *past* its size.
This means potentially freeing a nested stack of expansions
all at once.

Fixes gbdev#696
@Rangi42 Rangi42 force-pushed the expansion-nesting branch from f8c4a80 to 31f42d8 Compare April 17, 2021 17:03
@ISSOtm
Copy link
Member

ISSOtm commented Apr 17, 2021

'tis a surprisingly simple fix to an issue that seemed very complex to handle 🤷‍♂️

@Rangi42
Copy link
Contributor Author

Rangi42 commented Apr 17, 2021

I think it's only possible because now there's maximum 1 lookahead, and it only needs to be aware of one expansion nesting at a time. Previously ld [FF_OO PLUS_C], with nested expansions and extra whitespace in those two equs, would have been problematic.

@Rangi42 Rangi42 merged commit 9923fa3 into gbdev:master Apr 17, 2021
@Rangi42 Rangi42 deleted the expansion-nesting branch April 17, 2021 17:15
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 this pull request may close these issues.

Expanding recurse EQUS "recurse" or recurse EQUS "\{recurse\}" hangs rgbasm
2 participants