zsh (live version): setting globassign crashes completion

[Aleksandrina Nikolova]

Today I installed the latest git commit and was greeted by:
_main_complete:388: _lastcomp: assignment to invalid subscript range
after any completion I tried to do. Removing all configs (global and
local) fixed the problem so I started enabling things one by one to see
what is causing this. Bottom line:
a default setup with only:
autoload -Uz compinit
compinit
setopt globassign
gives the above stated error. Sorry, I don't have time to further
investigate, maybe someone else can look it up.

[Peter Stephenson]

On Tue, 2 Jun 2015 22:17:02 +1000

Post by Aleksandrina Nikolova
_main_complete:388: _lastcomp: assignment to invalid subscript range
after any completion I tried to do. Removing all configs (global and
local) fixed the problem so I started enabling things one by one to see
autoload -Uz compinit
compinit
setopt globassign
gives the above stated error. Sorry, I don't have time to further
investigate, maybe someone else can look it up.

We certainly shouldn't ever depend on the setting of the glob_assign
option within the completion scripts, so this fix is a sensible thing to
do anyway and ought to deal with it.

What is actually triggering the error is a bit obscure,
though... nothing obviously relevant has changed. I guess some new and
otherwise unproblematic assignment just picked up this alternative
meaning.

pws

diff --git a/Completion/compinit b/Completion/compinit
index 9470c92..79f9d42 100644
--- a/Completion/compinit
+++ b/Completion/compinit
@@ -142,6 +142,7 @@ _comp_options=(
NO_cshnullglob
NO_cshjunkiequotes
NO_errexit
+ NO_globassign
NO_globsubst
NO_histsubstpattern
NO_ignorebraces

[Mikael Magnusson]

On Tue, Jun 2, 2015 at 3:36 PM, Peter Stephenson

Post by Peter Stephenson
On Tue, 2 Jun 2015 22:17:02 +1000

Post by Aleksandrina Nikolova
_main_complete:388: _lastcomp: assignment to invalid subscript range
after any completion I tried to do. Removing all configs (global and
local) fixed the problem so I started enabling things one by one to see
autoload -Uz compinit
compinit
setopt globassign
gives the above stated error. Sorry, I don't have time to further
investigate, maybe someone else can look it up.

We certainly shouldn't ever depend on the setting of the glob_assign
option within the completion scripts, so this fix is a sensible thing to
do anyway and ought to deal with it.
What is actually triggering the error is a bit obscure,
though... nothing obviously relevant has changed. I guess some new and
otherwise unproblematic assignment just picked up this alternative
meaning.

My money is on
"33816, 33819: GLOB_ASSIGN changes integer and floating type variables
to string scalars"
I checked that my patch still passes the test added by those patches,
and it does.

[Peter Stephenson]

On Tue, 2 Jun 2015 15:45:09 +0200

Post by Mikael Magnusson
My money is on
"33816, 33819: GLOB_ASSIGN changes integer and floating type variables
to string scalars"
I checked that my patch still passes the test added by those patches,
and it does.

(Moved to zsh-workers.)

Ick.

We'd better do this, too.

pws

diff --git a/Test/E01options.ztst b/Test/E01options.ztst
index 5c453c8..195ce56 100644
--- a/Test/E01options.ztst
+++ b/Test/E01options.ztst
@@ -483,6 +483,14 @@

Post by Mikael Magnusson
tmpcd tmpfile1 tmpfile2
tmp*

+ (setopt globassign
+ typeset -A foo
+ touch gatest1 gatest2
+ foo=(gatest*)
+ print ${(t)foo})
+0:GLOB_ASSIGN doesn't monkey with type if not scalar assignment.
+>association-local
+
mkdir onlysomefiles
touch onlysomefiles/.thisfile onlysomefiles/thatfile
setopt globdots

[Mikael Magnusson]

On Tue, Jun 2, 2015 at 4:00 PM, Peter Stephenson

Post by Peter Stephenson
On Tue, 2 Jun 2015 15:45:09 +0200

Post by Mikael Magnusson
My money is on
"33816, 33819: GLOB_ASSIGN changes integer and floating type variables
to string scalars"
I checked that my patch still passes the test added by those patches,
and it does.

(Moved to zsh-workers.)
Ick.
We'd better do this, too.
pws
diff --git a/Test/E01options.ztst b/Test/E01options.ztst
index 5c453c8..195ce56 100644
--- a/Test/E01options.ztst
+++ b/Test/E01options.ztst
@@ -483,6 +483,14 @@

Post by Mikael Magnusson
tmpcd tmpfile1 tmpfile2
tmp*

+ (setopt globassign
+ typeset -A foo
+ touch gatest1 gatest2
+ foo=(gatest*)
+ print ${(t)foo})
+0:GLOB_ASSIGN doesn't monkey with type if not scalar assignment.
+>association-local
+
mkdir onlysomefiles
touch onlysomefiles/.thisfile onlysomefiles/thatfile
setopt globdots

Hm, there's one GLOB_ASSIGN test in A06 and one in E01, should we move
them together? Also, my patch had some unrelated gunk in the context,
sorry about that.

[Peter Stephenson]

On Tue, 2 Jun 2015 16:21:25 +0200

Post by Mikael Magnusson
Hm, there's one GLOB_ASSIGN test in A06 and one in E01, should we move
them together? Also, my patch had some unrelated gunk in the context,
sorry about that.

A06 might be better --- the other tests are more closely relevant ---
but we could leave a note in E01 that this has happened to avoid
future duplication.

pws

[Peter Stephenson]

diff --git a/Test/A06assign.ztst b/Test/A06assign.ztst
index 0ad9a0a..a4401cb 100644
--- a/Test/A06assign.ztst
+++ b/Test/A06assign.ztst
@@ -419,14 +419,14 @@

worldliness
world

- integer i n x
+ (integer i n x
float f
setopt globassign
i=tmpfile1
- n=tmp*
+ n=tmpf*
x=*2
f=2+2
- typeset -p i n x f
+ typeset -p i n x f)
0:GLOB_ASSIGN with numeric types

typeset -i i=0
typeset -a n

@@ -434,6 +434,25 @@

typeset x=tmpfile2
typeset -E f=4.000000000e+00

+ setopt globassign
+ foo=tmpf*
+ print $foo
+ unsetopt globassign
+ foo=tmpf*
+ print $foo
+0:GLOB_ASSIGN option
+>tmpfile1 tmpfile2
+>tmpf*
+
+ (setopt globassign
+ typeset -A foo
+ touch gatest1 gatest2
+ foo=(gatest*)
+ print ${(t)foo}
+ rm -rf gatest*)
+0:GLOB_ASSIGN doesn't monkey with type if not scalar assignment.
+>association-local
+
A=(first second)
A="${A[*]}" /bin/sh -c 'echo $A'
print -l "${A[@]}"
diff --git a/Test/E01options.ztst b/Test/E01options.ztst
index 5c453c8..d64f7ac 100644
--- a/Test/E01options.ztst
+++ b/Test/E01options.ztst
@@ -473,15 +473,7 @@

outside2 scalar
inside3 scalar-export

- setopt globassign
- foo=tmp*
- print $foo
- unsetopt globassign
- foo=tmp*
- print $foo
-0:GLOB_ASSIGN option
->tmpcd tmpfile1 tmpfile2
->tmp*
+# GLOB_ASSIGN is tested in A06assign.ztst.

mkdir onlysomefiles
touch onlysomefiles/.thisfile onlysomefiles/thatfile

[Mikael Magnusson]

---

isstr is set to (WC_ASSIGN_TYPE(ac) == WC_ASSIGN_SCALAR) a bit above this.
Previously, when setopt globassign was in effect, this would happen,
% typeset -A foo
% foo=(*)
% echo ${(t)foo}
array

when it should still say association, this is what caused _lastcomp to
change type and break completion.

Src/exec.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Src/exec.c b/Src/exec.c
index 007447d..2e996d4 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -2264,14 +2264,14 @@ addvars(Estate state, Wordcode pc, int addflags)
state->pc = opc;
return;
}
- if (!isstr || (isset(GLOBASSIGN) &&
+ if (!isstr || (isset(GLOBASSIGN) && isstr &&
haswilds((char *)getdata(firstnode(vl)), 0))) {
globlist(vl, 0);
/* Unset the parameter to force it to be recreated
* as either scalar or array depending on how many
* matches were found for the glob.
*/
- if (isset(GLOBASSIGN))
+ if (isset(GLOBASSIGN) && isstr)
unsetparam(name);
}
if (errflag) {