search

OpenTTD / nml

NewGRF Meta Language
GNU General Public License v2.0
44 stars 36 forks source link

Attempt to warn when use of ternary op might fail due to side effects #241

Closed andythenorth closed 3 years ago

andythenorth commented 3 years ago

Discovered via https://gist.github.com/andythenorth/64f3532ee4c148c5d7f9bf21ba36cda1#file-gistfile1-txt

FIRS code using procedures and storage access in a ternary op appeared to be incorrectly evaluating as true

e.g. foo() < bar() ? a : b was returning True for a case where value of foo() and bar() should have been 12 and 9, e.g. 12 < 9 ? a : b returned a.

glx diagnosed this as a case where temp register 0x100 (?) is re-used within the chain, which causes incorrect results.

The fix is to not use ternary op in these cases. glx has a commit which might function as a warning in these cases.

glx22 commented 3 years ago

No the issue is not the reuse of temp register 0x100, but the way ternary are compiled.

For guard ? expr_true : expr_false, first guard is evaluated, then stored as guard, and it's opposite stored as !guard. Then some magic happens for the result, it's translated as !guard * expr_false + guard * expr_true, meaning both expressions are evaluated in any case.

If any of both expression writes to a register, you have an unexpected side effect.