SUMMARY KCalc produces wrong results with bit operations starting with unary minus STEPS TO REPRODUCE 1. Enter the sequence - 58 >> 2 OBSERVED RESULT The computed value is -14 EXPECTED RESULT The computed value is -15 SOFTWARE/OS VERSIONS Linux/KDE Plasma: Arch @ Linux 6.14.4 KDE Plasma Version: 6.3.4 KDE Frameworks Version: 6.13.0 Qt Version: 6.9 ADDITIONAL INFORMATION Poking around a little, its quite clear that kcalc computes "-(58 >> 2)", not "(-58) >> 2" (the later which does produce the correct result)
I was poking around in the source code, it seems like the sequence of events leading to the bug is: 1. Expression starting with - is encountered, and a leading zero is inserted to make the binary - operator happy (in `CalcEngine::calculate()` 2. Expression is evaluated backwards in `CalcEngine::reduce_Stack_` I'm happy to submit a patch fixing it, but after debugging a little the right solution was no longer obvious. I see two possible approaches: 1: Further modify the special case handling of first token being a `TokenCode::MINUS` in `CalcEngine::calculate()` to ensure -number is always parsed as (-number). Which I think requires keeping state across iterations... which seems somewhat distasteful 2: Rewrite `CalcEngine` to support operators precedence and handle operators that can both be binary and unary. What I think is the _right_ solution and I'm happy to do so, but its a major refactor for a random drive-by patch. I'm happy to do implement either (or just leave the problem for somebody else), but I need somebody to point me in the right direction first.
(In reply to Johannes from comment #1) > I was poking around in the source code, it seems like the sequence of events > leading to the bug is: > > 1. Expression starting with - is encountered, and a leading zero is inserted > to make the binary - operator happy (in `CalcEngine::calculate()` > 2. Expression is evaluated backwards in `CalcEngine::reduce_Stack_` > > I'm happy to submit a patch fixing it, but after debugging a little the > right solution was no longer obvious. I see two possible approaches: > > 1: Further modify the special case handling of first token being a > `TokenCode::MINUS` in `CalcEngine::calculate()` to ensure -number is always > parsed as (-number). Which I think requires keeping state across > iterations... which seems somewhat distasteful > 2: Rewrite `CalcEngine` to support operators precedence and handle operators > that can both be binary and unary. What I think is the _right_ solution and > I'm happy to do so, but its a major refactor for a random drive-by patch. > > I'm happy to do implement either (or just leave the problem for somebody > else), but I need somebody to point me in the right direction first. Hello Johannes, I am the developer of that code, so, if you enter (- 58) >> 2 you get the expected -15, when I added all the operators I pretty much decided (for convenience) that the binary operators will take precedence over "unary left operators" and this is result of that, I entered the same in other places (Qalculate, the C compiler) and seems to be behaving like what you are expecting, implementing this would not be easy tho, if you want to work in this open an issue in the git repo so we can discuss how to approach it.
(In reply to Gabriel Barrantes from comment #2) > tho, if you want to work in this open an issue in the git repo so we can > discuss how to approach it. That would be https://invent.kde.org/utilities/kcalc/-/issues ? (I'm new around here, I thought _this_ was the place) But yeah, attention and free time willing, I'm perfectly happy to take a swing at this.
> That would be https://invent.kde.org/utilities/kcalc/-/issues ? (I'm new > around here, I thought _this_ was the place) Yes, right there