This bug appeared on fbc version 1.01.0 (probably the 2014-09-27)
Un simple bug example is when the #define directive with a trailing space is used twice inside a same expression of the code body:
#define false 0 '<- trailing space after the directive
Print false + false
Compiler output:
error 88: Recursive DEFINE not allowed, found '+' in 'Print false + false'
Referring to forum at post:
http://www.freebasic.net/forum/viewtopic.php?p=202144#p202144
and the followings.
The bug appears with commit [b1beb2] - Fix self-assignment type mismatch detection
I believe the significant changes are in parser-expr-binary.bas.
Related
Commit: [b1beb2]
This issue has kept me thinking for almost 3 days now, but that's probably just me worrying too much... it should be fixed in Git now though - [380919].
It looks like commit [51d974] (parser: Check for self-assignments when parsing BOPs for disambiguation) introduced the issue. In my test, if I remember correctly, it still worked with the previous commit, [b1beb2] (Fix self-assignment type mismatch detection).
The problem is with the interaction of token look-ahead and the lexer's macro recursion check. It makes sense too - that self-assignment parsing commit added a lot of look-ahead to the expression parser, and the lexer's recursive macro checking is limited to say the least (actually it's fundamentally broken since it only tracks the most toplevel macro, not the entire expansion stack).
Turns out that the lexer's "current macro" status was not updated regularly enough. Only during lexSkipToken(), but not when reading further tokens via look-ahead. This meant that after expanding a macro, if we encounter a consecutive (non-recursive) expansion during token look-ahead, it was disallowed as if it was recursive.
That's exactly the kind of situation introduced into the expression parser by that commit, lots of lexGetLookAhead() before lexSkipToken(). Of course, that just exposed the issue in the lexer, and indeed it seems like similar issues can be reproduced with older fbc versions. For example:
and in theory, anything for which the parser uses lexGetLookAhead() would be affected too...
So this should be fixed now, and luckily it looks like we don't need to rewrite the lexer just yet, and neither do we need to revert commit [51d974]. Without a fix for this I would have reverted that change, because breaking macro expansion in expression like that is not good at all.
As a bonus, it seems like nested (but not recursive) expansions like the following will finally work too:
I sure hope I didn't break anything [again] though. At least the changes only affect the macro recursion check, not macro expansion itself.
Related
Commit: [380919]
Commit: [51d974]
Commit: [b1beb2]