Skip to content

Refactor lexing of fixed-point numbers#1915

Merged
Rangi42 merged 7 commits into
gbdev:masterfrom
Rangi42:fixed-point-mantissa
Apr 7, 2026
Merged

Refactor lexing of fixed-point numbers#1915
Rangi42 merged 7 commits into
gbdev:masterfrom
Rangi42:fixed-point-mantissa

Conversation

@Rangi42
Copy link
Copy Markdown
Contributor

@Rangi42 Rangi42 commented Apr 4, 2026

readFractionalPart was a messy state machine with unclear correspondence to the actual fixed-point literals we accept and reject, so I refactored it.

This also distinguishes between "no digits after q" and "only 0s after q", so we no longer need to error about "significant digits".

I also renamed nonDigit to prevSeparator in other number-parsing functions, not just the fixed-point ones.

@Rangi42 Rangi42 added this to the 1.0.2 milestone Apr 4, 2026
@Rangi42 Rangi42 requested a review from ISSOtm April 4, 2026 22:15
@Rangi42 Rangi42 added rgbasm This affects RGBASM refactoring This PR is intended to clean up code more than change functionality labels Apr 4, 2026
@Rangi42
Copy link
Copy Markdown
Contributor Author

Rangi42 commented Apr 4, 2026

There's a pre-existing bug here: precision = precision * 10 + (c - '0'); can overflow a uint8_t, so that e.g. 0.5q264 gets treated like 0.5q8. I'll fix that separately after this PR, since this is just a refactor. Filed as #1917 for now.

Comment thread src/asm/lexer.cpp Outdated
Copy link
Copy Markdown
Member

@ISSOtm ISSOtm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess breaking apart a state machine into straight-line code is a readability win. Nice job!

Comment thread src/asm/lexer.cpp Outdated
Comment thread src/asm/lexer.cpp Outdated
Comment thread src/asm/lexer.cpp Outdated
@Rangi42 Rangi42 self-assigned this Apr 4, 2026
@Rangi42 Rangi42 requested a review from ISSOtm April 5, 2026 20:11
Comment thread src/asm/lexer.cpp
Comment thread src/asm/lexer.cpp Outdated
Comment thread src/asm/lexer.cpp Outdated
@Rangi42 Rangi42 requested a review from ISSOtm April 5, 2026 22:06
@Rangi42 Rangi42 assigned ISSOtm and unassigned Rangi42 Apr 5, 2026
Rangi42 added 2 commits April 5, 2026 22:37
…edPoint`

This incidentally fixes a bug with too-long fixed-point literals
that have precision suffixes.
Copy link
Copy Markdown
Member

@ISSOtm ISSOtm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some more refactoring ideas. Let me know if you think this is getting too bikeshed-y.

Comment thread src/asm/lexer.cpp
Comment thread src/asm/lexer.cpp
Comment thread src/asm/lexer.cpp
@Rangi42 Rangi42 requested a review from ISSOtm April 7, 2026 01:03
Copy link
Copy Markdown
Member

@ISSOtm ISSOtm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pulling through this! It was more painful than anticipated, huh.

Comment thread src/asm/lexer.cpp
@Rangi42 Rangi42 merged commit 11f6278 into gbdev:master Apr 7, 2026
24 checks passed
@Rangi42 Rangi42 deleted the fixed-point-mantissa branch April 7, 2026 01:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactoring This PR is intended to clean up code more than change functionality rgbasm This affects RGBASM

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants