KSP-KOS / KOS

Fully programmable autopilot mod for KSP. Originally By Nivekk
Other
691 stars 229 forks source link

Strange evaluation order #2631

Open zroug opened 4 years ago

zroug commented 4 years ago

I would expect the first print statement to print 42 and the second to print 0 but both statements print 0.

I suspect that this is related to #2152.

local x to 42.
print x + ({set x to 0. return x.}):call().
set x to 42.
print ({set x to 0. return x.}):call() + x.
Dunbaratu commented 4 years ago

It's not the order of operations. It's the fact that when you say "x" that's just a variable name string until it decides to evaluate it, and it didn't decide to evaulate it until it was part of some other expression like the "+".

It's more a matter of order of dereferencing, meaning "When does 'the thing on the stack is x, which is a variable you'll have to look up later' become 'I've looked it up now, forget the variable, it's now the numerical value on the stack.'? That is happening "late" in kOS because it has to for other operations in the kerboscript late-binding language.

zroug commented 4 years ago

I question if in this case x shouldn't be evaluated sooner (when it is mentioned). The current order can lead to very unintuitive behavior when dealing with anonymous functions. For example given the identity function and the example above:

function id {
    parameter x.
    return x.
}

Then

local x to 42.
print x + ({set x to 0. return x.}):call().

is very different from

local x to 42.
print id(x) + ({set x to 0. return x.}):call().

. That is why I referenced #2152. I think the pusheval instruction that is proposed there would move the time when x is evaluated to a more intuitive point.

NooneAtAll commented 4 years ago

There's an easier explanation to the shown results

if, like in many languages,
C = A() + B()
says nothing about whether A() or B() is evaluated first (only that those are evaluated before +), then in your examples, lambda execution can happen earlier (with x becoming 0) or later (with x staying 42)
And only compiler would know which of them happens where

zroug commented 4 years ago

I know some purely functional languages that I think don't define the order of argument evaluation but you can't observe this because they don't allow any side effects. All other languages that I know of evaluate arguments from left to right.

Of course the example that I have given is only a minimized toy example. Here is a similar example that is a little bit more believable:

local currentState to 0.

function updateState {
    set currentState to currentState + 1.
    return currentState.
}

function doSomething {
    parameter oldState, newState.
}

doSomething(currentState, updateState()).

// instead of
// local oldState to currentState.
// local newState to updateState().
// doSomething(oldState, newState).

Personally I came across this issue when I wanted to add caching to an existing program to improve performance. Another use case where argument evaluation order might be relevant is logging.

It turns out that KerboScript already evaluates arguments almost always from left to right. The only exception is that literal variable names are evaluated last. That feels a little bit inconsistent, so much that I thought this was a bug.

If it is intended behavior I wonder if this should be changed. If you don't want to change this I'm perfectly fine with that. I just think it would be little bit more intuitive.

NooneAtAll commented 4 years ago

All other languages that I know of evaluate arguments from left to right.

C, C++, Golang, Rust - all have order of evaluation either "undefined" (any result allowed) or "unspecified" (any evaluation order allowed)

And if we look through kOS documentation, order of evaluation is not mentioned there.
So, officially, this is not a bug

Whether it would be a nice thing to have (since it gets the language closer to Python) is up to devs

zroug commented 4 years ago

Interesting. Somehow I have always taken the order as given and didn't think that much about it. I don't even know why. Maybe because most languages that I have used do that. For rust I am the most surprised because I'm familiar with the language and this is something I would have expected to be defined considering the design of the rest of the language. And it looks like they have indeed freezed the evaluation order but that is not officially documented yet. There is an open issue to do that.

But like you said: It is up to the devs what to do for KerboScript.

nuggreat commented 4 years ago

Most languages don't have a specific a order or execution because some of the compiler optimizations will change the order of execution on you to improve performance. The thing they do instead is preserve the mathematical validity of the result meaning that no mater what order the compiler changes the operations to you will still get the same result out the other side.

zroug commented 4 years ago

That is for sure but I wasn't talking about semantic preserving transformations.

Dunbaratu commented 4 years ago

I think kOS is evaluating left-to-right. It's just not de-referencing variables when pushing them onto the stack. It does so when popping them from the stack. ("X" is just a variable name, until you use it for something then it becomes its value).

To test this, @zroug, can you re-do your original example with this one change and see what it does? Change this line:

print x + ({set x to 0. return x.}):call().

To This line:

print (x+0) + ({set x to 0. return x.}):call().

By placing the "x" inside another expression like that, it will have to get popped and evaluated before the lefthand side of the other plus sign is done. If I'm right - that change will prove it's left-to-right (you will now get the 42 on the lefthand side, not the 0 you would get if it was right-to-left.)

Essentially the fix would have to involve two kinds of PUSH operator - PUSH and PUSHEVAL. The reason that PUSH does not eval (but POP does) is that when the variable name is on the lefthand side of an assignment, it needs to see the variable itself on the stack so it can be assigned-into, not the value (with the variable it came from forgotten about, which is what an eval does). The fix would involve having "push X as an lval" and push X as an rval" be using two different opcodes, one that does the eval and one that does not.

The other fix, faster to code but I dislike it for having unnecessary execution steps, is to say, whenever doing a math expression, to always append an EVAL opcode after both the left and right operand. Thus X+Y, which was PUSH X, PUSH Y, ADD would become instead PUSH X, EVAL, PUSH Y, EVAL, ADD.

While I think having two different pushes - one with an implied eval in it and one without, is the more elegant solution, it's also the one more frought with potential pitfalls needing regression testing - because there may very well be plenty of other places where the code was depending on how Push doesn't always Eval. If so, all such places would need to be found and dealt with.