Fix incorrect lexing of "$ff00+c" #882
Conversation
Fixes gbdev#881 by moving the task from the lexer to the parser. This both alleviates the need for backtracking in the lexer, removing what is (was) arguably a hack, and causes tokenization boundaries to be properly respected, fixing the issue mentioned above.
ISSOtm
added
bug
|
We tried similar fixes some time ago, and they produced grammar conflicts; I did not get any on my machine (Bison 3.7.6), so I'm making this PR to check that this is also the case on other platforms. |
src/asm/parser.y
Outdated
| | T_LBRACK relocexpr T_OP_ADD T_MODE_C T_RBRACK { | ||
| int32_t val = rpn_GetConstVal(&$2); | ||
|
|
||
| if (val != 0xff00) { |
There was a problem hiding this comment.
To avoid potentially two error messages here, I would do:
if (!rpn_isKnown(&$2) || $2.val != 0xff00)
error("Expected constant expression equal to $FF00 for \"$ff00+c\"");
There was a problem hiding this comment.
(The "(did you mistype the C?)" is also unusual for RGBDS error messages.)
There was a problem hiding this comment.
Unusual, certainly, but is that a bad thing? This is a weird edge case of the parser, so guiding users into likely error causes should be a win, right?
There was a problem hiding this comment.
I think the "equal to $FF00" is clear enough if they try something like ld [$c200+c], a.
There was a problem hiding this comment.
I'm considering
cd = 420
ld [$C200 + c], a ; Oops, I forgot the `d` in `cd`!There was a problem hiding this comment.
Then they look at the line and see the +c typo. bg equ $42 / ld [$c200+b], a just says "syntax error, unexpected b".
There was a problem hiding this comment.
I wouldn't really mind if "did you mistype the C?" were very likely to be helpful advice, but it's misleading in the cases where the not-$ff00-number is the mistake.
|
This does appear to produce no conflicts, go figure. Maybe some grammar changes? :S |
Fixes #881 by moving the task from the lexer to the parser.
This both alleviates the need for backtracking in the lexer, removing what is (was) arguably a hack, and causes tokenization boundaries to be properly respected, fixing the issue mentioned above.