integers do not work as boolean flags in UNTIL's (update: and maybe all other uses of Opcode LogicalNot).

[Dunbaratu]

integers do not work as boolean flags in UNTIL's (update: and maybe all other uses of Opcode LogicalNot).

[Dunbaratu]

I am not in a position at the moment to edit the code. My usual laptop with all the development stuff is in for repairs right now, but I very strongly suspect the culprit is this, if someone else wants to implement the fix:

The error happens with UNTIL but not with IF because of the fact that UNTIL makes use of the opcode LogicalNot, while IF doesn't. The actual error is inside OpcodeLogicalNot. In the case where the operand is a number rather than a boolean, OpcodeLogicalNot is failing to invert the boolean meaning of the value.

In the file "Compilation/Opcode.cs", OpcodeLogicNot.execute() has these lines:

            if (value is bool)
                result = !((bool)value);
            else if (value is int)
                result = -((int)value);
            else if (value is double)
                result = -((double)value);

If the input was 1 (or any other nonzero number), then the output should be 0. But the code above would make it a -1, which is still true.

Similarly, if the input is 0, the output should be 1 (or any other nonzero number). But the above code would leave it at 0. (negative zero), making no change.

This is my suggested replacement for that code section. Only the two lines marked with the comment "// changed line" are different:

            if (value is bool)
                result = !((bool)value);
            else if (value is int)
                result = ((int) ( (value==0) ? 1: 0 ) );  // changed line.
            else if (value is double)
                result = ((double) ( (value==0.0) ? 1.0 : 0.0 ) ); // changed line.

[marianoapp]

That won't work because (regardless of the name) the opcode OpcodeLogicNot is also responsible for negating numbers, so it's used in expressions like "-(2*3)".

There are two ways to solve this:

• Add a new opcode to branch if the expression is true (therefore not needing to use OpcodeLogicNot)
• In the compiler code for the UNTIL statement add an opcode OpcodeLogicToBool before the OpcodeLogicNot.

I'll look into this once I'm back from work.

[Dunbaratu]

Ahh, that explain Issue #29 then. The meaning of arithmetic negation and the meaning of logical not have been conflated together at a very deep level of the RISC instruction set.

I would much prefer the following third way to solve it, as it solves other problems too and makes the design cleaner and more correct:

SO this third way to fix it is to stop trying to overload boolean nots with arithmetic negation. They are two entirely different operations.

It's better to allow a script writer to write:

set x to not x.
versus
set x to -x.

to make it clear which was meant. Use the minus sign for its intended purpose of arithmetic negation, and not for boolean inversion. (And then if you try to use the minus sign for booleans after this update, it's an error. I doubt this will break anyone's code because nobody' is going to have been in the habit of using the minus sign for boolean nots. It's not a thing a person would guess to try.)

I definitely do not like keeping the name "OpcodeLogicNot" when that's not what the opcode actually does. It's misleadingly named. If it's a dual-purpose opcode then it should be called something that reflects that. (or, even better, have two different opcodes, one for negation and one for not.)

In fact, I feel so passionately about this that I'm willing to put in the work to implement this as two different opcodes myself if you'll wait a bit for my computer to be repaired (probably by tomorrow morning).

RISC is a nice principle, but in this case it looks like it was taken to the extreme to the point where it caused distinct mathematical operations to be conflated in a buggy way.

[Dunbaratu]

I suppose another way to fix it if you don't want to define any new opcodes is to insert a call to OpcodeLogicToBool not just here, but everywhere a logical operation occurs. I'd say that if you do that, though, that the name of OpcodeLogicNot should be changed to reflect the fact that that's not really what it does if it also does double-duty as an arithmetic negater.

[marianoapp]

They have to be to separate opcodes, the current implementation is just something temporal I threw together during development and forgot about it.

[Dunbaratu]

Okay I'm actually very glad you said that, because a different opcode for logical not versus arithmetic negation would be my preferred solution too. It's much cleaner.

I just got my computer back from the shop so I should be able to look into this tonight.

[Dunbaratu]

Now addressed with pull request #46 instead of #38.

[marianoapp]

Closing as this was fixed in pull request #46.