Optimizing "(string='a') | (string = 'b')" fails
Original Reporter info from Mantis: MichaelKarcher
-
Reporter name: Michael Karcher
Original Reporter info from Mantis: MichaelKarcher
- Reporter name: Michael Karcher
Description:
The optimization introduced in r14714 (optimizing boolean expressions with two identical operands) applies only for short-circuited operation, because in non-short-circuit mode, you can count on the number of terms executed. It probably is an oversight that this optimization only applies if the short-circuit flag is set locally (which only happens for the macpas | and & operators, never for the "or" and "and" operators), but not if the global boolean evaluation strategy is set to "short-circuit evaluation". As the optimization is limited to the macpas syntax, real-world impact of this optimization is probably quite low.
A problem with this optimizer is that is transforms "(string='a') or (string='b')" into "(string='a')". The reason is that it uses parse-tree equivalence of the two expressions, and parse-tree equivalence is implemented through the node comparison operation. For string nodes, only the length and the assembler labels are compared, as equivalence of labels is equivalent to equivalence of context. The boolean expression optimization happens before label allocation takes place, so the label is "nil" for both 'a' and 'b', and thus 'a' and 'b' compare equal.
Steps to reproduce:
Compile the attached "fpcbugcheck.pas" with fpc past r14714. Observe the output "Mac: fpc failed!"
Additional information:
The whole transformation seems dubious to me, as I (with a C background, not a pascal background) assume short-circuited booleans to operate as in C, so something like:
if (ReadKey = 'x') and (ReadKey = 'x') then WriteLn('You entered two xs')
prints the message only if 'x' was entered twice, and only eats one non-x-Character (supposing short-circuit is on) if less than two xs are entered.
I attach one patch that enables this dubious optimization also for short-circuited 'or' and 'and', so the transformation gets much more testing with the Pascal Code in the wild, and a second patch that falls back to byte buffer comparison if labels are both nil.
Mantis conversion info:
- Mantis ID: 21255
- OS: All
- Platform: All
- Version: 2.6.1
- Fixed in version: 3.0.0
- Fixed in revision: 20485 (#d1acb76d)