Alt_Radar refactor? (was: This small script creates stack overflow)

[ZiwKerman]

    function foo
    {
        declare parameter ar.

        print ar.
    }

    lock alt_radar to alt:radar.

    until ag1
    {
        ar(alt_radar).
        wait 0.001.

        //vecdrawargs(ship:position, sv, RGB(1,0,0),"Surface Vel", 5.0, true).
    }

Log:

[LOG 12:16:39.799] kOS: At test_bug on archive, line 8
lock alt_radar to alt:radar.
                  ^
Called from test_bug on archive, line 12
    ar(alt_radar).
       ^

[LOG 12:16:39.799] System.Exception: Stack overflow!!
  at kOS.Execution.Stack.Push (System.Object item) [0x00000] in <filename unknown>:0 
  at kOS.Execution.CPU.PushStack (System.Object item) [0x00000] in <filename unknown>:0 
  at kOS.Execution.CPU.PushAboveStack (System.Object thing) [0x00000] in <filename unknown>:0 
  at kOS.Safe.Compilation.OpcodeCall.Execute (ICpu cpu) [0x00000] in <filename unknown>:0 
  at kOS.Execution.CPU.ExecuteInstruction (kOS.Execution.ProgramContext context) [0x00000] in <filename unknown>:0 
[LOG 12:16:39.802] Code Fragment
File                 Line:Col IP   opcode operand
----                 ----:--- ---- --------------------- 
                        0:0   0000 jump 19 
archive/test_bug        2:1   0001 pushscope 1 0 
archive/test_bug        3:23  0002 push ar 
archive/test_bug        3:23  0003 swap 
archive/test_bug        3:23  0004 storelocal 
archive/test_bug        5:5   0005 push $<argstart> 
archive/test_bug        5:11  0006 push $ar 
archive/test_bug        5:11  0007 call print() 
archive/test_bug        5:11  0008 pop 
archive/test_bug        6:1   0009 popscope 1 
archive/test_bug        6:1   0010 push 0 
archive/test_bug        6:1   0011 return 
archive/test_bug        8:1   0012 pushscope 2 0 
archive/test_bug        8:19  0013 push $<argstart> 
archive/test_bug        8:19  0014 call $alt_radar* <<--INSTRUCTION POINTER--
archive/test_bug        8:19  0015 popscope 1 
archive/test_bug        8:19  0016 return 
archive/test_bug        8:1   0017 push $alt_radar 
archive/test_bug        8:1   0018 return 
archive/test_bug        8:1   0019 nop 
archive/test_bug        1:1   0020 push $foo* 
archive/test_bug        1:1   0021 push 1 
archive/test_bug        1:1   0022 store 
archive/test_bug        8:1   0023 push $alt_radar* 
archive/test_bug        8:1   0024 pushdelegate 12 
archive/test_bug        8:1   0025 storeglobal 
archive/test_bug       10:7   0026 push $ag1 
archive/test_bug       10:7   0027 not 
archive/test_bug       10:7   0028 br.false 11 
archive/test_bug       11:1   0029 pushscope 3 0 
archive/test_bug       12:7   0030 push $<argstart> 
archive/test_bug       12:8   0031 push $<argstart> 
archive/test_bug       12:8   0032 call $alt_radar* 
archive/test_bug       12:8   0033 call ar 
archive/test_bug       12:8   0034 pop 
archive/test_bug       13:10  0035 push 0.001 
archive/test_bug       13:10  0036 wait 
archive/test_bug       16:1   0037 popscope 1 
archive/test_bug       16:1   0038 jump -12 
archive/test_bug       17:0   0039 nop 
                        0:0   0040 EOP 

[LOG 12:16:39.818] kOS: Stack dump: stackPointer = 1000
2994      kOS.Safe.Execution.VariableScope
          ScopeId=3, ParentScopeId=0, ParentSkipLevels=1 IsClosure=False
2993      SubroutineContext: {CameFromInstPtr 33}
2992      kOS.Safe.Execution.VariableScope
          ScopeId=2, ParentScopeId=0, ParentSkipLevels=1 IsClosure=False
2991      SubroutineContext: {CameFromInstPtr 15}
2990      kOS.Safe.Execution.VariableScope
          ScopeId=2, ParentScopeId=0, ParentSkipLevels=1 IsClosure=False
2989      SubroutineContext: {CameFromInstPtr 15}
2988      kOS.Safe.Execution.VariableScope
          ScopeId=2, ParentScopeId=0, ParentSkipLevels=1 IsClosure=False
2987      SubroutineContext: {CameFromInstPtr 15}
2986      kOS.Safe.Execution.VariableScope
          ScopeId=2, ParentScopeId=0, ParentSkipLevels=1 IsClosure=False
2985      SubroutineContext: {CameFromInstPtr 15}
2984      kOS.Safe.Execution.VariableScope
          ScopeId=2, ParentScopeId=0, ParentSkipLevels=1 IsClosure=False
2983      SubroutineContext: {CameFromInstPtr 15}
2982      kOS.Safe.Execution.VariableScope
          ScopeId=2, ParentScopeId=0, ParentSkipLevels=1 IsClosure=False
2981      SubroutineContext: {CameFromInstPtr 15}
2980      kOS.Safe.Execution.VariableScope
          ScopeId=2, ParentScopeId=0, ParentSkipLevels=1 IsClo
... and so on

[Dunbaratu]

Are you aware that "ALT:RADAR" isn't a real suffix (there is no "ALT" object) and instead there's a tricky alias where when you type ALT:RADAR it turns it into ALT_RADAR"?

So saying LOCK ALT_RADAR TO ALT:RADAR is effectively infinite recursion of the form LOCK A TO A.

I was going to mark this as not-a-bug but then I realized... maybe it might be possible to detect the LOCK A TO A case in the compiler. I could set a variable curLockName to the identifier's name when compiling the lock's expression, and have it throw an error when it notices that you are trying to use that identifier name somewhere inside the expression.

[ZiwKerman]

For some reason the lock alone does not produce overflow, only when you try to pass it as a parameter

so this code lock alt_radar to alt:radar executes without any problems

[Dunbaratu]

I see nowhere in your code where function foo() actually gets called.

[ZiwKerman]

hmm, even calling non-existent funstion ar causes overflow

[Dunbaratu]

That's because you never got that far. First it evaluates that arguments, puts the result on the stack, THEN calls ar(). It never got far enough to tell that ar() isn't a function because it hit the infinite recursion when trying to eval alt_radar to get the argument ready.

You'd have had the same problem if you did anything else with alt_radar:

[code] lock alt_radar to alt:radar. print alt_radar. [/code] Would do it too.

[abenkovskii]

So ALT_RADAR isn't a valid identifier. 1) Is it documented? 2) What is the reason behind doing it this way? Why can't we use something that isn't a valid kerboscript identifier in cases like this?

[Dunbaratu]

1) I hate the fact that ALT:RADAR is fake alias for ALT_RADAR. It's not my decision to do it that way. That's very old original design and now hard to change without breaking everything.

2) Because you have to be able to use LOCK on bound variables. i.e. LOCK THROTTLE.

It may be possible to add more checks to detect if a bound variable is get-only and not allow it in a lock.

[abenkovskii]

Could you change alt to be a structure or at least alt:radar to be an alias to alt-radar along with next ksm-breaking update?

[Dunbaratu]

That may be possible. The use of ALT_RADAR was never documented so the fact that it works was never made into a public promise. (Although I did use it in my hover PID example because I didn't realize it was undocumented - I should go fix that).

[abenkovskii]

Making alt a structure will enable people to pass it as a parameter, a return value, or just assign it to a variable. Don't know is it useful or not.

[abenkovskii]

How many other implicit aliases like ALT_RADAR are out there?

[erendrake]

@abenkovskii the only one i know of is ETA

[abenkovskii]

@erendrake What about VERSION?

[erendrake]

@abenkovskii nope, VersionInfo is a structure

https://github.com/KSP-KOS/KOS/blob/6b7d217b11dfed690da105d369083d1a1da1c220/src/kOS.Safe/Encapsulation/VersionInfo.cs

[Dunbaratu]

If you're talking about fixing alt:radar we should fix the fact that it's presumed to only work on the current ship. You can get target:altitude but not target:alt_radar.

I am of the opinion that ALL the shorthands like ALTITUDE, PROGRADE, etc, need to be aliases to SHIP:thingy. Many are , but alt radar is one of the holdouts.

[Dunbaratu]

after my other fix, does this still happen? Can it be closed?

[abenkovskii]

1. The original program contained a bug: ar(alt_radar). is supposed to be foo(alt_radar).
2. Both versions now don't cause stack overflow
3. The original rises "undefined variable name *ar"
4. The fixed version works fine.

This issue is fixed now.