completion problem with '291' ok with '274'.

completion problem with '291' ok with '274'.

[Ray Andrews]

    $ /usr/local/bin<< change to where all my builds are.
    zsh-5.0.7-291-gda86d6b  << most recent build here.

    $ ./zsh<TAB>  << no luck
    $ ./zsh<TAB> << no luck
    $ ./zsh<TAB>-5.0.7-<TAB> ... << ok 3d time, but it was as likely to
    crash the xterm.

    $ ./zsh-5.0.7-274-gce211bb << try an older build.
    zsh-5.0.7-274-gce211bb


... no completion problems with '274'. I went back and forth between 
'291' and '274' and every time '291' was unstable completing, whereas 
'274' was fine. Most often '291' caused the xterm to crash.

Binary here is all static link, but plain vanilla otherwise.

Re: completion problem with '291' ok with '274'.

[Bart Schaefer]

On Feb 10,  2:36pm, Ray Andrews wrote:
}
}     $ /usr/local/bin<< change to where all my builds are.
}     zsh-5.0.7-291-gda86d6b  << most recent build here.
} 
}     $ ./zsh<TAB>  << no luck

Mind telling us whether this is "zsh -f" and/or if you've run compinit
and/or what zstyles are in play?

}     $ ./zsh<TAB> << no luck

Chances are both of these are because there actually is a binary named
"zsh" in the current directory, so it's the first possible completion.

}     $ ./zsh<TAB>-5.0.7-<TAB> ... << ok 3d time, but it was as likely to
}     crash the xterm.

FYI "crash the xterm" is not really what happens.  More likely zsh has
crashed and the xterm simply exited because the only process that it
was running has exited.  Try using "xterm -hold" so the window stays
open until explicitly closed, then you'll be able to see any error
messages that are generated.  If the xterm goes away even with -hold,
then you actually are somehow killing the xterm rather than zsh and
the problem isn't ours ...

} ... no completion problems with '274'. I went back and forth between 
} '291' and '274' and every time '291' was unstable completing, whereas 
} '274' was fine. Most often '291' caused the xterm to crash.

Could be related to workers/34485, but you may not have directory write
permission to drop a core file in /usr/local/bin so you'll have to run
from inside gdb to get a stack trace.

I can't repro with zsh-5.0.7-293-g0209635 but it's doubtful anything in
between would have fixed a completion crash.

Re: completion problem with '291' ok with '274'.

[Ray Andrews]

On 02/10/2015 06:35 PM, Bart Schaefer wrote:
> Mind telling us whether this is "zsh -f" and/or if you've run compinit
> and/or what zstyles are in play?

No, just plain 'zsh'.  I'm running my standard setup, not sure what 
details are relevant.  compinit yes, as always.

styles:

--------------------------------------------------------------------------------------------------------

zstyle ':completion:*:default' list-colors ${(s.:.)LS_COLORS}
zstyle ':completion:*:match:*' original only
zstyle ':completion:*:approximate:*' max-errors 1 numeric
zstyle ':completion:*:expand:*' tag-order all-expansions
zstyle ':completion:*' completer _expand _complete
zstyle ':completion:*' auto-description 'specify: %d'
zstyle ':completion:*' format 'Completing %d'
zstyle ':completion:*' group-name ''
zstyle ':completion:*' list-colors ''
zstyle ':completion:*' list-prompt %SAt %p: Hit TAB 'for more', or the 
char to insert%s
zstyle ':completion:*' menu select=2
zstyle ':completion:*' menu select=long
zstyle ':completion:*' select-prompt %SScrolling active: current 
selection at %p%s
zstyle ':completion:*' use-compctl false
zstyle ':completion:*' verbose true
zstyle ':completion:*:cd:*' ignore-parents parent pwd
zstyle ':completion:*:killall:*' command 'ps -u $USER -o cmd'
zstyle ':completion:*:kill:*' force-list always
zstyle ':completion:*:kill:*' command 'ps -u $USER -o 
pid,%cpu,tty,cputime,cmd'
zstyle ':completion:*:*:kill:*' menu yes select
zstyle ':completion:*:*:kill:*:processes' list-colors '=(#b) 
#([0-9]#)*=0=01;31'
zstyle ':completion:*' matcher-list '' 'm:{a-z}={A-Z}' 
'm:{a-zA-Z}={A-Za-z}' 'r:|[._-]=* r:|=* l:|=*'
--------------------------------------------------------------------------------------------------------

> Chances are both of these are because there actually is a binary named
> "zsh" in the current directory, so it's the first possible completion.

Correct, but 291 'newlines' and then crashes, whereas 274 (tho I have to 
press TAB twice to get past the 'zsh' which is just a link to the 
current binary) stays on the same line and then completes without 
trouble. One sees the list of possible completions, and all is as it 
should be.
> FYI "crash the xterm" is not really what happens.  More likely zsh has
> crashed and the xterm simply exited because the only process that it
> was running has exited.

Ok.
> Try using "xterm -hold" so the window stays
> open until explicitly closed, then you'll be able to see any error
> messages that are generated.  If the xterm goes away even with -hold,
> then you actually are somehow killing the xterm rather than zsh and
> the problem isn't ours ...

I'm actually using 'xfce4-terminal' which seems not to have the '-hold' 
ability.  Starting that from within itself the results are the same, the 
terminal evaporates, leaving the calling terminal. With 'xterm -hold' 
any attempt at completion of anything freezes the xterm solid.  I kill 
it by closing the window. No message. Plain 'xterm' does the same as 
xfce4-terminal--it evaporates back to the calling terminal.
>
> } ... no completion problems with '274'. I went back and forth between
> } '291' and '274' and every time '291' was unstable completing, whereas
> } '274' was fine. Most often '291' caused the xterm to crash.
>
> Could be related to workers/34485, but you may not have directory write
> permission to drop a core file in /usr/local/bin so you'll have to run
> from inside gdb to get a stack trace.

That's new territory for me, so I'll need a primer.

> I can't repro with zsh-5.0.7-293-g0209635 but it's doubtful anything in
> between would have fixed a completion crash.
293 is the same as 291.

274 is still perfect.

Everything is fine with 'zsh -f' (with 293). So then this is something 
specific to my configuration?  Should I narrow it down by elimination?
>

Re: completion problem with '291' ok with '274'.

[Bart Schaefer]

On Feb 10,  7:39pm, Ray Andrews wrote:
}
} No, just plain 'zsh'.  I'm running my standard setup, not sure what 
} details are relevant.  compinit yes, as always.
} 
} styles:

OK, with those settings the first thing you should see (on the first TAB)
is something like:

torch% ./zsh
Completing executable file or directory
zsh*              zsh-5.0.3*        zsh-5.0.5*        zsh.old*
zsh-5.0.2-dev-0*  zsh-5.0.4*        zsh-5.0.7*

The second TAB does nothing because it is selecting the first match in
the list, which is just "zsh".  The third tab should advance to the
second match in the list (which in my case is "zsh-5.0.2-dev-0").

It's possible (but unlikely) that something in your LS_COLORS is having
this effect.  Any change if you remove that first list-colors zstyle?

} I'm actually using 'xfce4-terminal' which seems not to have the '-hold' 

It's -H or --hold with two leading hyphens (there's your inconsistency
again ... X11/Xorg apps standardize on long names with a single hyphen,
other apps use the GNU convention of double hyphens).

} > Could be related to workers/34485, but you may not have directory write
} > permission to drop a core file in /usr/local/bin so you'll have to run
} > from inside gdb to get a stack trace.
} 
} That's new territory for me, so I'll need a primer.

Should work to just do

	xfce4-terminal gdb /usr/local/bin/zsh-5.0.7-293-g0209635

and then at the "(gdb) " prompt, type "run".  Then when it crashes you'll
be back at the (gdb) prompt, type "where".

} Everything is fine with 'zsh -f'

That bypasses compinit and uses the old compctl, so not surprising.

Re: completion problem with '291' ok with '274'.

[Ray Andrews]

On 02/10/2015 08:20 PM, Bart Schaefer wrote:
> OK, with those settings the first thing you should see (on the first TAB)
> is something like:
>
> torch% ./zsh
> Completing executable file or directory
> zsh*              zsh-5.0.3*        zsh-5.0.5*        zsh.old*
> zsh-5.0.2-dev-0*  zsh-5.0.4*        zsh-5.0.7*
>
> The second TAB does nothing because it is selecting the first match in
> the list, which is just "zsh".  The third tab should advance to the
> second match in the list (which in my case is "zsh-5.0.2-dev-0").

Right, it's like that with 274 but with 291 the first TAB gives me a new 
prompt or it crashes. I understand the 'nothing' TABS.

     #autoload -U compinit && compinit

... and all is well.


     autoload -U compinit && compinit
     return

... and I still have the problem, (the return skips all 'style' lines.)
> It's -H or --hold with two leading hyphens (there's your inconsistency
> again ... X11/Xorg apps standardize on long names with a single hyphen,
> other apps use the GNU convention of double hyphens).

I'll stick with real xterm for now so that there's no confusion. 
(Somehow, that inconsistency doesn't bother me, I'd not be expecting 
consistency there.  That's worth thinking about.  Presumptuous double 
standard?)

Should work to just do

	xfce4-terminal gdb /usr/local/bin/zsh-5.0.7-293-g0209635

and then at the "(gdb) " prompt, type "run".  Then when it crashes you'll
be back at the (gdb) prompt, type "where".

Output:

Starting program: /usr/local/bin/zsh-5.0.7-293
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/i386-linux-gnu/i686/cmov/libthread_db.so.1".

                                                                                     TAB here:
pts/5 HP-w5--5-Debian1 root /usr/local/bin 3$ l zsh[Inferior 1 (process 19073) exited normally]
(gdb) where
No stack.
(gdb)

Re: completion problem with '291' ok with '274'.

[Bart Schaefer]

On Feb 10, 10:10pm, Ray Andrews wrote:
}
}      autoload -U compinit && compinit
}      return
} 
} ... and I still have the problem, (the return skips all 'style' lines.)

Curious.

} Starting program: /usr/local/bin/zsh-5.0.7-293
} [Thread debugging using libthread_db enabled]
} Using host libthread_db library "/lib/i386-linux-gnu/i686/cmov/libthread_db.so.1".
} 
}                                                                                      TAB here:
} pts/5 HP-w5--5-Debian1 root /usr/local/bin 3$ l zsh[Inferior 1 (process 19073) exited normally]

Now *that* is strange.  Let's try this.  At the first (gdb) prompt, before
you do "run", type "break zexit".  If it really is exiting normally that
should stop the process before it actually does exit, so "where" will be
able to display something.

Re: completion problem with '291' ok with '274'.

[Ray Andrews]

On 02/11/2015 08:28 AM, Bart Schaefer wrote:
> Now *that* is strange.  Let's try this.  At the first (gdb) prompt, before
> you do "run", type "break zexit".  If it really is exiting normally that
> should stop the process before it actually does exit, so "where" will be
> able to display something.
...

Type "apropos word" to search for commands related to "word"...
Reading symbols from ./zsh-5.0.7-293...(no debugging symbols found)...done.
(gdb) break zexit
Function "zexit" not defined.
Make breakpoint pending on future shared library load? (y or [n]) n
(gdb) run
Starting program: /usr/local/bin/zsh-5.0.7-293
[Thread debugging using libthread_db enabled]
Using host libthread_db library 
"/lib/i386-linux-gnu/i686/cmov/libthread_db.so.1".

$   << back to prompt, no chance for 'where'.

I hope this isn't some hideous local anomaly :(

Re: completion problem with '291' ok with '274'.

[Bart Schaefer]

[-- Attachment #1: Type: text/plain, Size: 274 bytes --]

On Feb 11, 2015 9:40 AM, "Ray Andrews" <rayandrews@eastlink.ca> wrote:
>
> Reading symbols from ./zsh-5.0.7-293...(no debugging symbols
found)...done.

Aha.  "make install" has stripped the symbol table from the binary.  Can
you use the binary from your build tree instead?

Re: completion problem with '291' ok with '274'.

[Ray Andrews]

On 02/11/2015 12:54 PM, Bart Schaefer wrote:
> On Feb 11, 2015 9:40 AM, "Ray Andrews" <rayandrews@eastlink.ca> wrote:
>> Reading symbols from ./zsh-5.0.7-293...(no debugging symbols
> found)...done.
>
> Aha.  "make install" has stripped the symbol table from the binary.  Can
> you use the binary from your build tree instead?
>
I don't do a 'make install', just 'make' and 'make check'.  Mind, this 
is with 'disable-dynamic' (sp?) and that custom config file you sent, 
however 274 was built the same way and it's fine, so that seems an 
unlikely clue.  I just tried a 'make install' on a lark but it changed 
nothing.  Binary size is 1264492 if that is any hint of some compile issue.

Re: completion problem with '291' ok with '274'.

[Bart Schaefer]

n Feb 11,  8:00pm, Bart Schaefer wrote:
}
} I'm not sure about the exec.c change that moved the "if (do_exec)" block,
} but I'm wondering whether it has to do with Ray's mysterious shell exits,
} because it makes an _exit() call.

And sure enough ...

On Feb 11,  9:06pm, Ray Andrews wrote:
} Subject: Re: completion problem with '291' ok with '274'.
}
} Breakpoint 2, _exit () at ../sysdeps/unix/sysv/linux/i386/_exit.S:24
} 24    ../sysdeps/unix/sysv/linux/i386/_exit.S: No such file or directory.
} (gdb) where
} #0  _exit () at ../sysdeps/unix/sysv/linux/i386/_exit.S:24
} #1  0xb7dda0e7 in __run_exit_handlers (status=status@entry=0,
}      listp=0xb7f503c4 <__exit_funcs>,
}      run_list_atexit=run_list_atexit@entry=true) at exit.c:97
} #2  0xb7dda17d in __GI_exit (status=0) at exit.c:104
} #3  0x080667a3 in execcmd (state=0xbfffa8a0, input=0, output=0, how=18,
}      last1=1) at exec.c:3494

So basically we have to back out all of 34485 and start that over.

Here's the reverse diff for 34485:

diff --git a/Src/exec.c b/Src/exec.c
index 9bbcf49..3b0e936 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -2427,7 +2427,6 @@ execcmd(Estate state, int input, int output, int how, int last1)
     wordcode code;
     Wordcode beg = state->pc, varspc;
     FILE *oxtrerr = xtrerr, *newxtrerr = NULL;
-    LinkList restorelist = 0, removelist = 0;
 
     doneps4 = 0;
     redir = (wc_code(*state->pc) == WC_REDIR ? ecgetredirs(state) : NULL);
@@ -3360,9 +3359,9 @@ execcmd(Estate state, int input, int output, int how, int last1)
 		zcontext_restore();
 	    } else
 		redir_prog = NULL;
-
+	    
 	    lastval = execfuncdef(state, redir_prog);
-	}
+	} 
 	else if (type >= WC_CURSH) {
 	    if (last1 == 1)
 		do_exec = 1;
@@ -3375,6 +3374,7 @@ execcmd(Estate state, int input, int output, int how, int last1)
 	    } else
 		lastval = (execfuncs[type - WC_CURSH])(state, do_exec);
 	} else if (is_builtin || is_shfunc) {
+	    LinkList restorelist = 0, removelist = 0;
 	    /* builtin or shell function */
 
 	    if (!forked && ((cflags & BINF_COMMAND) ||
@@ -3424,6 +3424,29 @@ execcmd(Estate state, int input, int output, int how, int last1)
 		} else
 		    clearerr(stdout);
 	    }
+	    if (isset(PRINTEXITVALUE) && isset(SHINSTDIN) &&
+		lastval && !subsh) {
+#if defined(ZLONG_IS_LONG_LONG) && defined(PRINTF_HAS_LLD)
+		fprintf(stderr, "zsh: exit %lld\n", lastval);
+#else
+		fprintf(stderr, "zsh: exit %ld\n", (long)lastval);
+#endif
+		fflush(stderr);
+	    }
+
+	    if (do_exec) {
+		if (subsh)
+		    _exit(lastval);
+
+		/* If we are exec'ing a command, and we are not in a subshell, *
+		 * then check if we should save the history file.              */
+		if (isset(RCS) && interact && !nohistsave)
+		    savehistfile(NULL, 1, HFILE_USE_OPTIONS);
+		exit(lastval);
+	    }
+	    if (restorelist)
+		restore_params(restorelist, removelist);
+
 	} else {
 	    if (!forked)
 		setiparam("SHLVL", --shlvl);
@@ -3473,28 +3496,6 @@ execcmd(Estate state, int input, int output, int how, int last1)
 		execlist(state, 0, 1);
 	    }
 	}
-	if (isset(PRINTEXITVALUE) && isset(SHINSTDIN) &&
-	    lastval && !subsh) {
-#if defined(ZLONG_IS_LONG_LONG) && defined(PRINTF_HAS_LLD)
-	    fprintf(stderr, "zsh: exit %lld\n", lastval);
-#else
-	    fprintf(stderr, "zsh: exit %ld\n", (long)lastval);
-#endif
-	    fflush(stderr);
-	}
-
-	if (do_exec) {
-	    if (subsh)
-		_exit(lastval);
-
-	    /* If we are exec'ing a command, and we are not in a subshell, *
-	     * then check if we should save the history file.              */
-	    if (isset(RCS) && interact && !nohistsave)
-		savehistfile(NULL, 1, HFILE_USE_OPTIONS);
-	    exit(lastval);
-	}
-	if (restorelist)
-	    restore_params(restorelist, removelist);
     }
 
   err:
diff --git a/Src/parse.c b/Src/parse.c
index ffd25de..0b54a90 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -1612,7 +1612,8 @@ par_funcdef(int *cmplx)
 	    num++;
 	    zshlex();
 	}
-	*cmplx = 1;
+	if (num > 0)
+	    *cmplx = 1;
 	ecbuf[parg] = ecused - parg; /*?*/
 	ecbuf[parg+1] = num;
     }
@@ -1896,7 +1897,8 @@ par_simple(int *cmplx, int nr)
 		    argc++;
 		    zshlex();
 		}
-		*cmplx = 1;
+		if (argc > 0)
+		    *cmplx = 1;
 		ecbuf[parg] = ecused - parg; /*?*/
 		ecbuf[parg+1] = argc;
 	    }
diff --git a/Test/E01options.ztst b/Test/E01options.ztst
index 3213534..46b1837 100644
--- a/Test/E01options.ztst
+++ b/Test/E01options.ztst
@@ -783,24 +783,14 @@
 >print is a shell builtin
 ?(eval):8: command not found: print
 
-# PRINTEXITVALUE only works if shell input is coming from standard input.
-# Goodness only knows why.
-  $ZTST_testdir/../Src/zsh -f <<<'
-      setopt printexitvalue
-      func() {
-	  false
-      }
-      func
-  '
-1:PRINT_EXIT_VALUE option
-?zsh: exit 1
-
-  $ZTST_testdir/../Src/zsh -f <<<'
-      setopt printexitvalue
-      () { false; }
-  '
-1:PRINT_EXIT_VALUE option for anonymous function
-?zsh: exit 1
+# This option seems to be problematic.  I don't quite know how it works.
+##   func() {
+##     setopt localoptions printexitvalue
+##     false
+##   }
+##   func
+## 1:PRINT_EXIT_VALUE option
+## ?(eval):2: exit 1
 
   setopt promptbang
   print -P !

Re: completion problem with '291' ok with '274'.

[Peter Stephenson]

On Wed, 11 Feb 2015 21:30:54 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> So basically we have to back out all of 34485 and start that over.

Yes, it's job control stuff that the test suite doesn't cover.

The only completely safe way of fixing the print_exit_value problem
would be to duplicate that higher up.  We can restrict the effect of the
patch in other ways but it covers too much obscure stuff to be sure
about it.

Removing the tests in front of the "*cmplx = 1" must be safe in the
sense that if it shows up problems for annonymous functions then they're
already present in the standard execution path and hence need fixing.
However, with the rest of the change absent I'm not aware of remaining
side effects of this alone.

pws

Re: completion problem with '291' ok with '274'.

[Bart Schaefer]

On Feb 12,  9:25am, Peter Stephenson wrote:
} Subject: Re: completion problem with '291' ok with '274'.
}
} On Wed, 11 Feb 2015 21:30:54 -0800
} Bart Schaefer <schaefer@brasslantern.com> wrote:
} > So basically we have to back out all of 34485 and start that over.
} 
} Yes, it's job control stuff that the test suite doesn't cover.

So I should go ahead and commit that back-out?

} Removing the tests in front of the "*cmplx = 1" must be safe in the
} sense that if it shows up problems for annonymous functions then they're
} already present in the standard execution path and hence need fixing.

I thought we determined before that there's some sort of interaction
between the wordcode generation and the setting of *cmplx such that
one can't just change the way that value is computed in par_simple()
et al.  without making a corresponding change upstream?

I'm probably mis-remembering.

Re: completion problem with '291' ok with '274'.

[Peter Stephenson]

On Thu, 12 Feb 2015 08:43:23 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Feb 12,  9:25am, Peter Stephenson wrote:
> } Subject: Re: completion problem with '291' ok with '274'.
> }
> } On Wed, 11 Feb 2015 21:30:54 -0800
> } Bart Schaefer <schaefer@brasslantern.com> wrote:
> } > So basically we have to back out all of 34485 and start that over.
> } 
> } Yes, it's job control stuff that the test suite doesn't cover.
> 
> So I should go ahead and commit that back-out?

Yes, I think so.  Thanks.

> } Removing the tests in front of the "*cmplx = 1" must be safe in the
> } sense that if it shows up problems for annonymous functions then they're
> } already present in the standard execution path and hence need fixing.
> 
> I thought we determined before that there's some sort of interaction
> between the wordcode generation and the setting of *cmplx such that
> one can't just change the way that value is computed in par_simple()
> et al.  without making a corresponding change upstream?
> 
> I'm probably mis-remembering.

You can't in general set *cmplx to 0 because that takes you through the
simple path that doesn't handle everything.

You should certainly be able to set it to 1 to go through the
all-singing-all-dancing path.  As it does that in any case with
arguments, to fix an early problem, if it fails without arguments, then
something *very* weird is going on.  (Not saying it isn't, mind...)

pws