Modulus operator ignores sign when right-hand-side is const

[Jay0]

The Modulus operator in zscript (%) ignores the sign of the left-hand-side operand when the right-hand-side operand is a constant, performing a signed modulus operation even when it should perform an unsigned one.

Example: <uint larger than INT_MAX>%constant will be negative, while <uint larger than INT_MAX>%<another uint> will not

[Graf Zahl]

Hm. I fear there isn't much I can do now - the problem is so deep in the parser, filtering up several levels from there causing follow-up issues with insufficient type checks, that trying to change it will surely break more things than fix.

The biggest obstacle is that from the token scanner up to the evaluation stage it uses 32 bit constants, but screws up the constant type at the very lowest level already. And it's not just ZScript depending on this but all parsing code in the entire engine.

This entire code will have to be redesigned for 64 bit throughout to avoid these problems, but that's a major task of refactoring.

[randi]

PR#1691

[Nash]

Looks like these fixes broke the INIFile library - I'm too tired to look into this properly at the moment but I do know that library uses a lot of unsigned ints in its ZScript code. I've at least found which functions fail, have documented it in the other thread.

[randi]

I was worried this might be a risky change after all this time. How widespread is the problem of code using uints in a way where they rely on them being downgraded to ints? It might be best to make this version dependent.

[Graf Zahl]

Done. In the process I discovered another bug, i.e. that constant evaluation never set the engine version and essentially produced undefined behavior when entering version dependent code.

[Graf Zahl]

I'm not sure how much of a problem this may become - but I had to make more changes than I would have liked to compile the internal scripts with this change. One thing that was necessary to add is a warning for signed/unsigned comparisons because they may behave in very unintuitive ways if negative values get in. For that reason I had to change the type of Array::Size to signed, because this flooded the console with warnings and in a few times produced logic errors where checks with -1 were done.