[BUG] Anonymous functions cause funcfiletrace to produce function-relative line numbers

[BUG] Anonymous functions cause funcfiletrace to produce function-relative line numbers

[dana]

Hello again. Sorry for spamming the list, i can't seem to help myself.

I was experimenting with line-number tracing today and i found that anonymous
functions have an undesirable effect on funcfiletrace:

  % cat -n tracetest.zsh
       1  #
       2  foo() {
       3    #
       4    #
       5    #
       6    #
       7    #
       8    () { () { print -rC2 -- $functrace $funcfiletrace } }
       9  }
      10  foo
  % zsh tracetest.zsh
  (anon):0          tracetest.zsh:6
  foo:6             tracetest.zsh:8
  tracetest.zsh:10  tracetest.zsh:10

Note that the top call from (anon) is listed as tracetest.zsh:6 (actually the
relative line number within the function foo) rather than the expected
tracetest.zsh:8 (the absolute line number within the file).

I looked at the parameter module very briefly but nothing stood out immediately.
Probably will be obvious to someone else why it does that.

dana

Re: [BUG] Anonymous functions cause funcfiletrace to produce function-relative line numbers

[Peter Stephenson]

On Sun, 10 Dec 2017 17:28:03 -0600
dana <dana@dana.is> wrote:
> I was experimenting with line-number tracing today and i found that anonymous
> functions have an undesirable effect on funcfiletrace:
> 
>   % cat -n tracetest.zsh
>        1  #
>        2  foo() {
>        3    #
>        4    #
>        5    #
>        6    #
>        7    #
>        8    () { () { print -rC2 -- $functrace $funcfiletrace } }
>        9  }
>       10  foo
>   % zsh tracetest.zsh
>   (anon):0          tracetest.zsh:6
>   foo:6             tracetest.zsh:8
>   tracetest.zsh:10  tracetest.zsh:10
> 
> Note that the top call from (anon) is listed as tracetest.zsh:6 (actually the
> relative line number within the function foo) rather than the expected
> tracetest.zsh:8 (the absolute line number within the file).

I don't think this is just anonymous functions; I think the problem is
the temptation to use nested functions is just greater with anonymous
functions.

Perhaps...?

pws

diff --git a/Src/exec.c b/Src/exec.c
index fc6d02d..03b7f3d 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -5669,11 +5669,11 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
 	funcsave->fstack.caller = funcstack ? funcstack->name :
 	    dupstring(funcsave->argv0 ? funcsave->argv0 : argzero);
 	funcsave->fstack.lineno = lineno;
+	funcsave->fstack.flineno = funcstack->flineno + shfunc->lineno;
 	funcsave->fstack.prev = funcstack;
 	funcsave->fstack.tp = FS_FUNC;
 	funcstack = &funcsave->fstack;
 
-	funcsave->fstack.flineno = shfunc->lineno;
 	funcsave->fstack.filename = getshfuncfile(shfunc);
 
 	prog = shfunc->funcdef;

Re: [BUG] Anonymous functions cause funcfiletrace to produce function-relative line numbers

[Peter Stephenson]

On Mon, 11 Dec 2017 09:41:16 +0000
Peter Stephenson <p.stephenson@samsung.com> wrote:
> Perhaps...?

Oops, try this instead.

diff --git a/Src/exec.c b/Src/exec.c
index fc6d02d..24d3c7e 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -5669,11 +5669,13 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
 	funcsave->fstack.caller = funcstack ? funcstack->name :
 	    dupstring(funcsave->argv0 ? funcsave->argv0 : argzero);
 	funcsave->fstack.lineno = lineno;
+	funcsave->fstack.flineno = shfunc->lineno;
+	if (funcstack)
+	    funcsave->fstack.flineno += funcstack->flineno;
 	funcsave->fstack.prev = funcstack;
 	funcsave->fstack.tp = FS_FUNC;
 	funcstack = &funcsave->fstack;
 
-	funcsave->fstack.flineno = shfunc->lineno;
 	funcsave->fstack.filename = getshfuncfile(shfunc);
 
 	prog = shfunc->funcdef;

Re: [BUG] Anonymous functions cause funcfiletrace to produce function-relative line numbers

[dana]

On 11 Dec 2017, at 04:12, Peter Stephenson <p.stephenson@samsung.com> wrote:
> Oops, try this instead.

This one does not seem to have been entirely successful.

The reason i encountered this is that i was playing with a 'panic' function that
prints a trace-back. Here's a test script that replicates the basic 'stack' i
had going before:

  panic() {
    print -rC2 -- $functrace $funcfiletrace
    exit 1
  }
  puts() {
    panic
  }
  main() {
    () { puts }
  }
  main

The output, after applying the patch:

  puts:1            tracetest.zsh:15
  (anon):0          tracetest.zsh:9
  main:1            tracetest.zsh:9
  tracetest.zsh:11  tracetest.zsh:11

The line number for the (anon) call is correct now, but the one for the puts
call is wrong — there aren't even 15 lines in the script.

With my real-world code that line number seemed to be the sum of the current
function-relative number and the previous file-absolute number. That isn't the
case in this example, but maybe it had something to do with the fact that my
functions were all in separate files, unlike the ones here. Or it could be a
coincidence i guess.

dana

Re: [BUG] Anonymous functions cause funcfiletrace to produce function-relative line numbers

[Peter Stephenson]

On Mon, 11 Dec 2017 15:57:06 -0600
dana <dana@dana.is> wrote:
> On 11 Dec 2017, at 04:12, Peter Stephenson <p.stephenson@samsung.com> wrote:
> > Oops, try this instead.
>   panic() {
>     print -rC2 -- $functrace $funcfiletrace
>     exit 1
>   }
>   puts() {
>     panic
>   }
>   main() {
>     () { puts }
>   }
>   main
> 
> The output, after applying the patch:
> 
>   puts:1            tracetest.zsh:15
>   (anon):0          tracetest.zsh:9
>   main:1            tracetest.zsh:9
>   tracetest.zsh:11  tracetest.zsh:11

Hmm... it looks like anonymous functions do need special handling after
all, for reasons I haven't put my finger on but will be down to the
different relationship between the point of definition and point of
execution compared with normal functions.

See if this helps...

BTW this also implements one of the (trivial in this case) bits I
proposed for Daniel's problem.

Needs a test; this is very easy to break.

pws

diff --git a/Src/exec.c b/Src/exec.c
index 664d790..a3ac3b1 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -5104,6 +5104,7 @@ execfuncdef(Estate state, Eprog redir_prog)
 	    LinkList args;
 
 	    anon_func = 1;
+	    shf->node.flags |= PM_ANONYMOUS;
 
 	    state->pc = end;
 	    end += *state->pc++;
@@ -5669,11 +5670,13 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
 	funcsave->fstack.caller = funcstack ? funcstack->name :
 	    dupstring(funcsave->argv0 ? funcsave->argv0 : argzero);
 	funcsave->fstack.lineno = lineno;
+	funcsave->fstack.flineno = shfunc->lineno;
+	if (funcstack && (shfunc->node.flags & PM_ANONYMOUS))
+	    funcsave->fstack.flineno += funcstack->flineno;
 	funcsave->fstack.prev = funcstack;
 	funcsave->fstack.tp = FS_FUNC;
 	funcstack = &funcsave->fstack;
 
-	funcsave->fstack.flineno = shfunc->lineno;
 	funcsave->fstack.filename = getshfuncfile(shfunc);
 
 	prog = shfunc->funcdef;
diff --git a/Src/zsh.h b/Src/zsh.h
index 24d06ba..22ae954 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -1875,6 +1875,7 @@ struct tieddata {
 #define PM_DONTIMPORT_SUID (1<<19) /* do not import if running setuid */
 #define PM_LOADDIR      (1<<19) /* (function) filename gives load directory */
 #define PM_SINGLE       (1<<20) /* special can only have a single instance  */
+#define PM_ANONYMOUS    (1<<20) /* (function) anonymous function            */
 #define PM_LOCAL	(1<<21) /* this parameter will be made local        */
 #define PM_SPECIAL	(1<<22) /* special builtin parameter                */
 #define PM_DONTIMPORT	(1<<23)	/* do not import this variable              */

Re: [BUG] Anonymous functions cause funcfiletrace to produce function-relative line numbers

[dana]

On 15 Dec 2017, at 04:21, Peter Stephenson <p.stephenson@samsung.com> wrote:
>See if this helps...

Thanks. After applying the new patch, the line numbers all look good with the
test case i provided, whether it's executed from a file or from stdin. I also
tried it with the functions in separate files, and they look correct there
too... unless you use ksh-style definitions.

Test case for that:

  % < panic
  panic() {
    print -rC2 -- $functrace $funcfiletrace
    exit 1
  }

  % < puts
  #
  puts() {
    #
    #
    panic
  }

  % < tracetest.zsh
  fpath=( . )
  autoload -Uz panic puts
  main() {
    () { puts }
  }
  main

  % ZDOTDIR=. Src/zsh tracetest.zsh
  puts:3           /Users/dana/.../zsh/puts:3
  (anon):0         tracetest.zsh:4
  main:1           tracetest.zsh:4
  tracetest.zsh:6  tracetest.zsh:6

The file line number at the top should be 5.

I tried a few 'real-world' cases and they matched the above — looks fine without
the ksh-style definition, but if it's there it uses the function-relative line
number. I'm guessing that's also related to the difference i mentioned seeing in
my last e-mail.

(I think i'll probably stop using ksh-style definitions....)

dana

Re: [BUG] Anonymous functions cause funcfiletrace to produce function-relative line numbers

[Bart Schaefer]

On Fri, Dec 15, 2017 at 10:45 AM, dana <dana@dana.is> wrote:
>
> tried it with the functions in separate files, and they look correct there
> too... unless you use ksh-style definitions.

Does the same thing happen whenever a function is defined like this --

>   % < puts
>   #
>   puts() {
>     #
>     #
>     panic
>   }

--  in an autoload file (zsh included)?  That is, if the function is
parsed as part of a larger function, so it isn't defined until the
surrounding function is executed, then I'm betting the line numbers
from the original file are lost.

Re: [BUG] Anonymous functions cause funcfiletrace to produce function-relative line numbers

[dana]

On 16 Dec 2017, at 15:10, Bart Schaefer <schaefer@brasslantern.com> wrote:
>Does the same thing happen whenever a function is defined like this --
>...
>--  in an autoload file (zsh included)?

I wasn't sure what you meant here, since i am auto-loading the functions defined
that way. Someone on IRC suggested maybe you meant something like this?

  % < foo
  #
  panic() {
    print -rC2 -- $functrace $funcfiletrace
    exit 1
  }
  puts() {
    panic
  }
  puts

  % < tracetest.zsh
  fpath=( . )
  autoload -Uz foo
  main() {
    () { foo }
  }
  main

If that's the case, i actually *don't* see the problem there (with pws's patch):

  puts:1           /Users/dana/.../zsh/foo:7
  foo:9            /Users/dana/.../zsh/foo:9
  (anon):0         tracetest.zsh:4
  main:1           tracetest.zsh:4
  tracetest.zsh:6  tracetest.zsh:6

All those numbers look correct to me.

I also repeated my previous test case, but with `autoload -Uk` instead of `-Uz`,
and i got the same result as before, so the problem evidently isn't limited to
the combination of zsh-style auto-loading + ksh-style definitions.

(Let me know if i've misinterpreted your question)

dana

Re: [BUG] Anonymous functions cause funcfiletrace to produce function-relative line numbers

[Peter Stephenson]

On Fri, 15 Dec 2017 12:45:47 -0600
dana <dana@dana.is> wrote:
> On 15 Dec 2017, at 04:21, Peter Stephenson <p.stephenson@samsung.com> wrote:
> >See if this helps...
> 
> Thanks. After applying the new patch, the line numbers all look good with the
> test case i provided, whether it's executed from a file or from stdin. I also
> tried it with the functions in separate files, and they look correct there
> too... unless you use ksh-style definitions.

I think this fits in with what I realised, which is that line numbers
for functions are wrong if they're in a nested scope.  The anonymous
functions that brought this up are naturally defined that way because
they're executed in other functions and hence defined there, whereas
typically functions are deifned in a top level scope --- except for
ksh-style, where they're defined as functions inside the autoload scope
and then executed.

What I haven't worked out is how to fix this.  I'm confused about how
the definition and execution hierarchies fit together.

If I don't get further I may commit the current change as a general
improvement on previous behaviour.

pws

Re: [BUG] Anonymous functions cause funcfiletrace to produce function-relative line numbers

[Peter Stephenson]

I think this might be going in the right direction.

We don't need PM_ANONYMOUS, and in fact we've tested before by using a
special name pointer, but I quite like it for future use anyway...

I tried it with the following...

#snip
# this is line 1

foo() {

bar() { () { (){ print -rC2 -- $functrace $funcfiletrace; } } }
}
foo
bar
#snip

and got

(anon):0     ./why.txt:5
bar:0        ./why.txt:5
./why.txt:8  ./why.txt:8
zsh:1        zsh:1

which looks an improvement.

Probably not much activity till the New Year now.

pws


diff --git a/Src/exec.c b/Src/exec.c
index 3678ef8..38cc24a 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -5089,7 +5089,11 @@ execfuncdef(Estate state, Eprog redir_prog)
 	shf->node.flags = 0;
 	/* No dircache here, not a directory */
 	shf->filename = ztrdup(scriptfilename);
-	shf->lineno = lineno;
+	shf->lineno =
+	    (funcstack && (funcstack->tp == FS_FUNC ||
+			   funcstack->tp == FS_EVAL)) ?
+	    funcstack->flineno + lineno :
+	    lineno;
 	/*
 	 * redir_prog is permanently allocated --- but if
 	 * this function has multiple names we need an additional
@@ -5109,6 +5113,7 @@ execfuncdef(Estate state, Eprog redir_prog)
 	    LinkList args;
 
 	    anon_func = 1;
+	    shf->node.flags |= PM_ANONYMOUS;
 
 	    state->pc = end;
 	    end += *state->pc++;
diff --git a/Src/zsh.h b/Src/zsh.h
index 24d06ba..22ae954 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -1875,6 +1875,7 @@ struct tieddata {
 #define PM_DONTIMPORT_SUID (1<<19) /* do not import if running setuid */
 #define PM_LOADDIR      (1<<19) /* (function) filename gives load directory */
 #define PM_SINGLE       (1<<20) /* special can only have a single instance  */
+#define PM_ANONYMOUS    (1<<20) /* (function) anonymous function            */
 #define PM_LOCAL	(1<<21) /* this parameter will be made local        */
 #define PM_SPECIAL	(1<<22) /* special builtin parameter                */
 #define PM_DONTIMPORT	(1<<23)	/* do not import this variable              */

Re: [BUG] Anonymous functions cause funcfiletrace to produce function-relative line numbers

[dana]

On 20 Dec 2017, at 11:39, Peter Stephenson <p.stephenson@samsung.com> wrote:
>I think this might be going in the right direction.

As far as anonymous functions go, this works well for me. Can't see any problems
with nesting them, having them span multiple lines, or anything. Regular
functions look fine too obv.

(Not sure if this was meant to address the ksh-definitions thing, but just to be
clear, it's still the same there.)

On 20 Dec 2017, at 11:39, Peter Stephenson <p.stephenson@samsung.com> wrote:
>Probably not much activity till the New Year now.

Happy holidays!

dana