Hi Winfried, On Wednesday, 2019-04-17 11:54:13 +0000, Winfried Donkers wrote:
Upon debugging IFS( 0, NA(), 1+0, "output" ) I came to the following preliminary conclusions: Before ScInterpreter::ScIfs_MS() is called, ScInterpreter::Interpret is called for argument '1+0' . That leads to a call to scInterpreter::CalculateAddSub( false ) and there the problem starts. 0 and 1 are popped and added, but after PushDouble( ::rtl::math::approxAdd( 0, 1 ) ) the raw stacktype is svError.
That happens because nGlobalError is still set from the previous NA() call, any Push...() pushes the error value instead of a result if nGlobalError is set. Which is desired to propagate the error, and normally doesn't happen this way (carrying over from the previous argument) unless an OpCode is part of IsErrFunc() (which ocIfs_MS and ocSwitch_MS are, likely because of similar reasons, but it's only a partial workaround), otherwise calculation ends with the error. The actual root cause here is that IFS() and SWITCH() are not compiled into jumps so only the needed result would be calculated, but instead all arguments are calculated beforehand as in usual function calls. We had that discussion some while ago (and there exists some bug about it), but I didn't get around to a different implementation. (right now I only remember I had some idea for it to use existing means to compile an IFS() call into some kind of nested or chained ocIf or ocChoose or some such, would have to dig it out of my mail archive). A workaround might be that for pre-calculated results of IFS() and SWITCH() nGlobalError is cleared before stepping on to the next argument, but that could get hairy determining the right spot when to do it, i.e. when the same level of calls is reached. Maybe the place with the IsErrFunc() handling in ScInterpreter::Interpret() can be used for that, though determining whether the current OpCode is in the argument level of IFS() or SWITCH() probably isn't too trivial. Not treating IFS() and SWITCH() as jumps of course code paths are unnecessarily executed and has other unwanted side effects (like there were cases where someone used STYLE() within but not the actual final result), so the best fix would actually be to change that. Eike -- GPG key 0x6A6CD5B765632D3A - 2265 D7F3 A7B0 95CC 3918 630B 6A6C D5B7 6563 2D3A
Attachment:
signature.asc
Description: PGP signature