Closed Stewmath closed 5 years ago
Ouch, I'll check this one out tomorrow...
I took a look into this issue earlier today. It looks like the whitespace between 'num' and '-1' is getting dropped in 'preprocess_file' due to the '-' character and the previous space check:
/* go back? */
if ((z == 3 || *input == ',') && *(output - 1) == ' ')
output--;
Resulting in the line being treated as '.REDEFINE num-1'. When it gets into the 'REDEFINE' directive the initial 'get_next_token' call extracts the entire 'num-1' in the final else case that only delimits on space, comma and 0x0A.
Not sure on a fix yet but I wanted to give a heads up on the source of the issue.
On Sat, Jun 4, 2016 at 11:34 PM, ssjason123 notifications@github.com wrote:
I took a look into this issue earlier today. It looks like the whitespace between 'num' and '-1' is getting dropped in 'preprocess_file' due to the '-' character and the previous space check: /* go back? / if ((z == 3 || input == ',') && *(output - 1) == ' ') output--; Resulting in the line being treated as '.REDEFINE num-1'. When it gets into the 'REDEFINE' directive the initial 'get_next_token' call extracts the entire 'num-1' in the final else case that only delimits on space, comma and 0x0A.
Not sure on a fix yet but I wanted to give a heads up on the source of the issue.
Thanks! :) Looking at the code it's surrounded by ifdefs and is not compiled for SPC700 or 65816 - I wonder what was the idea behind this...
— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/vhelin/wla-dx/issues/89#issuecomment-223765072, or mute the thread https://github.com/notifications/unsubscribe/AGHtUijaVvyCrGarfy8saj1G9VC4DGOaks5qIakugaJpZM4HZ-mL .
I looked into this a bit more on Sunday but the results were very odd.
I tried removing the #if block entirely and the tests all built successfully with the exception of the SPC700 linker_test setup.s failing on Line 123:
and a, [jee] + Y
Based on that I wrote some code for GB-Z80 and found that I could get the same error with odd spacing on an instruction expecting an add with the #if's removed:
LD HL, SP + $10
So I thought it impacted any opcode with an expected add operation until I was scanning the SPC700 opcode list and found:
AND A,!?+Y
I expected this to fail with the odd spacing in SPC700 linker_test setup.s Line 124:
and a, !foul + Y
But it was successful and produced the expected opcode. I need to dig into this some more, there has to be another place removing space on the SPC700 or doing some kind of opcode swapping at a later stage to resolve it. With the extra spacing I would expect it to fail or get handled by:
AND A,x
With 'x' being treated as a stack/expression.
More research! I found out why the 'and' opcode on Line 124 in the SPC700 linker_test setup.s file is working correctly when the preprocessor space logic removed. The stack_calculate function in stack.c has an #if block for SPC700 to resolve this specific case:
#ifdef SPC700
/* check if the computation is of the form "y+X" or "y+Y" and remove that "+X" or "+Y" */
if (q > 2 && si[q - 2].type == STACK_ITEM_TYPE_OPERATOR && si[q - 2].value == SI_OP_PLUS) {
if (si[q - 1].type == STACK_ITEM_TYPE_STRING && si[q - 1].string[1] == 0)
Any time a +X or +Y is found as the second and third arguments in the stack calculation they are removed. The removal of the +Y in this case moves the current token back to the '+' which allows the opcode logic to match the '+Y'.
I think the best solution would be to rewrite the opcode parsing to consume the spaces and remove the special case whitespace adjustment in the preprocess on add/subtract but that is a pretty huge task. An alternative could be to remove the special case preprocess whitespace and do the whitespace change per line when we get to the opcode parsing stage. I had played with something similar on my branch where I was playing with define substitution on the line before evaluating the opcode. It's not the cleanest solution but it would be a lot less work than a complete opcode parse rewrite.
I just checked in a sample change to my branch for solution 2 to move the whitespace change from the preprocess to before parsing the opcode. It exposed an issue with the token_stack that I am not too happy about though. Its a pretty complicated case to explain but it happens when the stack gets a few nested entries and the final desired position is outside of the range active buffer. The current code only allows it to calculate the offset up to the size of the current buffer. The whitespace code has a special case to handle it as long as the offset matches the end of the current buffer (which should be the only possibility from the define substitution code that runs before it).
Link to my branch if you want to test it out: https://github.com/ssjason123/wla-dx/tree/parserUpgrade
A quick and ugly workaround for this might be as follows:
.define ABC 0-1
I hope we can find a way to fix this bug...
Sorry, I forgot to fix this one to WLA DX v9.8 final... I hope it works now better when you use the latest 9.9a sources...
Have you guys been using v9.9? Can we close this issue now, is it working?
In my WIP cmake test branch (#199) where I have an test case for negative redefinition works at current master (and did not work in some version before it was fixed).
This gives me an error like this:
This is easily worked around with brackets however.