kevinlawler / kona

Open-source implementation of the K programming language
ISC License
1.36k stars 138 forks source link

Yet another recursion problem #556

Closed tavmem closed 4 years ago

tavmem commented 4 years ago

This issue was identifed by @bakul

In k2.8

$ rlwrap -n ./k
K 2.8 2000-10-10 Copyright (C) 1993-2000 Kx Systems
\ for help. \\ to exit.

 foo:{a:x; `0:("x=",$x),"\n"; {:[x=0; a:x; foo(x-1)]}a; `0:"x=",($x),", a=",($a),"\n"; a}
 foo(3)
x=3
x=2
x=1
x=0
x=0, a=0
x=1, a=1
x=2, a=2
x=3, a=3
3

In Kona

$ rlwrap -n ./k
kona      \ for help. \\ to exit.

 foo:{a:x; `0:("x=",$x),"\n"; {:[x=0; a:x; foo(x-1)]}a; `0:"x=",($x),", a=",($a),"\n"; a}
 foo(3)
x=3
x=2
x=1
x=0
x=0, a=0
type error

This issue is similar to issue #549. In both cases, the recursion occurs within a subfunction.

tavmem commented 4 years ago

It is simewhat different from issue #549: Here, when using the "_f" form you get

tavmem commented 4 years ago

Similar to issue #549, it works fine if you don't use a subfunction:

$ rlwrap -n ./k
kona      \ for help. \\ to exit.

  f:{a:x; `0:("x=",$x),"\n";  if[x=0; a:x]; if[~x=0; f(x-1)]; `0:"x=",($x),", a=",($a),"\n"; a}
  f(3)
x=3
x=2
x=1
x=0
x=0, a=0
x=1, a=1
x=2, a=2
x=3, a=3
3
tavmem commented 4 years ago

If you make this change to track when

for issue #549 , you get

g:{a:x; {:[x=0; a:x; g(0)]}a; a} g 2 ...tree ...tc ...tree ...tc 0

tavmem commented 4 years ago

If you force the "tree-clone" to always be passed by reference with this change

$ git diff
diff --git a/src/kx.c b/src/kx.c
index c38f6ad..2f82ddc 100644
--- a/src/kx.c
+++ b/src/kx.c
@@ -526,7 +526,7 @@ K vf_ex(V q, K g)
             cd(kV(fc)[CONJ]); kV(fc)[CONJ]=0;
             kV(fc)[DEPTH]++;
             I tt=0; DO(o->n, if(kC(o)[i]=='{') { tt=1; break; } )
-            if(!grnt || tt || kC(o)[0]=='[') fw=wd_(kC(o),o->n,&tree,fc); else { tc=kclone(tree); fw=wd_(kC(o),o->n,&tc,fc); }
+            if(0){ fw=wd_(kC(o),o->n,&tree,fc); } else { tc=kclone(tree); fw=wd_(kC(o),o->n,&tc,fc); }
             kV(f)[CACHE_WD]=fw; cd(fc); }
           #ifdef DEBUG
             if(stk1>5) { cd(g); kerr("stack"); R _n(); }

you get the correct result for both issues:

  f:{a:x; `0:("x=",$x),"\n"; {:[x=0; a:x; f(x-1)]}a; `0:"x=",($x),", a=",($a),"\n"; a}
  f 3
x=3
x=2
x=1
x=0
x=0, a=0
x=1, a=1
x=2, a=2
x=3, a=3
3
  g:{a:x; {:[x=0; a:x; g(0)]}a; a}
  g 2
2
tavmem commented 4 years ago

However, if you run all tests always passing "tree-clone" by reference you get 36 fails

$ ./k_test
t:0
t:50
t:100
t:150
t:200
t:250
t:300
t:350
t:400
t:450
t:500
t:550
...
t:750
t:800
t:850
t:900
t:950
t:1000
t:1050
t:1100
Test pass rate: 0.9667, Total: 1114, Passed: 1045, Skipped: 33, Failed: 36, Time: 0.273883s
fail
kona      \ for help. \\ to exit.

We need to improve the criteria for when to pass

tavmem commented 4 years ago

I forgot to mention that if you use "_f" while forcing "tree-clone", you continue to get the correct results, but (for some reason) still miss the intermediate reporting:

  f:{a:x; `0:("x=",$x),"\n"; {:[x=0; a:x; _f(x-1)]}a; `0:"x=",($x),", a=",($a),"\n"; a}
  f 3
x=3
x=3, a=3
3
  g:{a:x; {:[x=0; a:x; _f(0)]}a; a}
  g 2
2
tavmem commented 4 years ago

What a surprise !! In k2.8, the intermediate reporting also dissapears when using "_f"

$ rlwrap -n ./k
K 2.8 2000-10-10 Copyright (C) 1993-2000 Kx Systems
\ for help. \\ to exit.

  f:{a:x; `0:("x=",$x),"\n"; {:[x=0; a:x; f(x-1)]}a; `0:"x=",($x),", a=",($a),"\n"; a}
  f 3
x=3
x=2
x=1
x=0
x=0, a=0
x=1, a=1
x=2, a=2
x=3, a=3
3

  f:{a:x; `0:("x=",$x),"\n"; {:[x=0; a:x; _f(x-1)]}a; `0:"x=",($x),", a=",($a),"\n"; a}
  f 3
x=3
x=3, a=3
3

In this case, we have just what we want.

tavmem commented 4 years ago

This change seems to be quite a kluge ... but it works.

$ git diff
diff --git a/src/kx.c b/src/kx.c
index c38f6ad..1ffcb8d 100644
--- a/src/kx.c
+++ b/src/kx.c
@@ -526,7 +526,9 @@ K vf_ex(V q, K g)
             cd(kV(fc)[CONJ]); kV(fc)[CONJ]=0;
             kV(fc)[DEPTH]++;
             I tt=0; DO(o->n, if(kC(o)[i]=='{') { tt=1; break; } )
-            if(!grnt || tt || kC(o)[0]=='[') fw=wd_(kC(o),o->n,&tree,fc); else { tc=kclone(tree); fw=wd_(kC(o),o->n,&tc,fc); }
+            I ttt=0; DO(o->n-1, if(kC(o)[i]=='{' && kC(o)[i+1]==':') { ttt=1; break; } )
+            if(!ttt && (!grnt || tt || kC(o)[0]=='[')) fw=wd_(kC(o),o->n,&tree,fc);
+            else{ tc=kclone(tree); fw=wd_(kC(o),o->n,&tc,fc); }
             kV(f)[CACHE_WD]=fw; cd(fc); }
           #ifdef DEBUG
             if(stk1>5) { cd(g); kerr("stack"); R _n(); }

Will try to find a better fix later.