Inconsistency of GLOB_ASSIGN

Inconsistency of GLOB_ASSIGN

[Bart Schaefer]

Consider:

torch% print C*
Config
torch% print c*
config.h config.log config.modules config.modules.sh config.status
torch% setopt GLOB_ASSIGN
torch% integer x
torch% x=C*
torch% typeset -p x
typeset -i x=0
torch% x=c*
torch% typeset -p x
typeset -a x
x=(config.h config.log config.modules config.modules.sh config.status)
torch% 

Shouldn't the glob assignment to x have turned it into an array, or at
least into a non-integer scalar, even though there was only one match?

Yes, I know the reason is that "Config" got expanded first and then taken
as a variable name in arithmetic evaluation, therefore defaulting to the
value 0 which was assigned to x.  Still, the following seems wrong to me:

torch% integer x
torch% typeset -p x
typeset -i x=0
torch% x=*.h
zsh: bad floating point constant

Re: Inconsistency of GLOB_ASSIGN

[Peter Stephenson]

On Wed, 26 Nov 2014 21:57:25 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> Yes, I know the reason is that "Config" got expanded first and then taken
> as a variable name in arithmetic evaluation, therefore defaulting to the
> value 0 which was assigned to x.

Ick.

> Still, the following seems wrong to me:
> 
> torch% integer x
> torch% typeset -p x
> typeset -i x=0
> torch% x=*.h
> zsh: bad floating point constant

Yes, I think that if there are expansions using GLOB_ASSIGN --- for which I
think the test that the result is different from the original is good
enough --- we ought to treat it as either a scalar or an array
assignment.  The mixture is stupid.

It would be neater always to do an array assignment, in fact, but
the traditional behaviour is that if there was only one result the
assignment is scalar, and GLOB_ASSIGN is really there only for
tradition.

Arguably we should look at the type of the destination and if it's
numeric not do a glob at all.  I'd be happy with that, since:

integer x
x=3*3

probably doesn't mean "look for files beginning and ending in 3", but maybe it's making the whole thing even more complicated.

I will try to think of a prize to give anyone who can make any language
other than zsh report a floating point error when assigning an array to
an integer.

pws

Re: Inconsistency of GLOB_ASSIGN

[Bart Schaefer]

On Nov 27,  8:43pm, Peter Stephenson wrote:
} Subject: Re: Inconsistency of GLOB_ASSIGN
}
} On Wed, 26 Nov 2014 21:57:25 -0800
} Bart Schaefer <schaefer@brasslantern.com> wrote:
} > torch% x=*.h
} > zsh: bad floating point constant
} 
} I will try to think of a prize to give anyone who can make any language
} other than zsh report a floating point error when assigning an array to
} an integer.

It's assigning the scalar "config.h" to an integer ... if it were an
array, it'd change the type of the variable and never attempt math.
Nevertheless ...

Re: Inconsistency of GLOB_ASSIGN

[Bart Schaefer]

On Nov 27,  8:43pm, Peter Stephenson wrote:
} Subject: Re: Inconsistency of GLOB_ASSIGN
}
} It would be neater always to do an array assignment, in fact, but
} the traditional behaviour is that if there was only one result the
} assignment is scalar, and GLOB_ASSIGN is really there only for
} tradition.

That's too bad, because making it always be an array assignment is a
lot simpler than figuring out whether the glob expanded to something.
(I think.  Still pondering.)

There are also very few contexts where a single-element array and
a scalar behave differently.  The only one I can think of would be
subscript slices, but if you're glob-assigning to a variable and then
expecting to be able to subscript it as a string, you're already in
pretty strange territory.

One interesting thing is what happens with NO_NOMATCH.  A failed glob
still returns a single-element array in that case, whereas a string
containing no globbing characters is a scalar.

} Arguably we should look at the type of the destination and if it's
} numeric not do a glob at all.  I'd be happy with that, since:
} 
} integer x
} x=3*3
} 
} probably doesn't mean "look for files beginning and ending in 3"

The "happy" behavior is already the case for NO_GLOB_ASSIGN, so I
think it's OK to ignore the type of the variable when the value is
asserted to be a glob.

Re: Inconsistency of GLOB_ASSIGN

[Bart Schaefer]

NOTE:  The two patches included in this message are mutually exclusive.
Do not attempt to apply both of them on top of one another.

On Nov 27,  1:38pm, Bart Schaefer wrote:
}
} On Nov 27,  8:43pm, Peter Stephenson wrote:
} }
} } It would be neater always to do an array assignment, in fact, but
} } the traditional behaviour is that if there was only one result the
} } assignment is scalar
} 
} That's too bad, because making it always be an array assignment is a
} lot simpler than figuring out whether the glob expanded to something.

So in actuality, not too much different.  Below are two patches, pick
one ... both need the haswilds() test to preserve the behavior of
assigning a non-glob-string-that-looks-like-math to an integer/float.

Here is the patch for making the assigment always be an array:

--- 8< --- snip --- 8< ---
diff --git a/Src/exec.c b/Src/exec.c
index 02a8fe3..702b731 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -2243,8 +2243,11 @@ addvars(Estate state, Wordcode pc, int addflags)
 		state->pc = opc;
 		return;
 	    }
-	    if (isset(GLOBASSIGN) || !isstr)
+	    if (!isstr || (isset(GLOBASSIGN) &&
+			   haswilds((char *)getdata(firstnode(vl))))) {
 		globlist(vl, 0);
+		isstr = 0;
+	    }
 	    if (errflag) {
 		state->pc = opc;
 		return;
--- >8 --- snip --- >8 ---

And here is the patch for making the type scalar or array based on how
many results got returned:

--- 8< --- snip --- 8< ---
diff --git a/Src/exec.c b/Src/exec.c
index 02a8fe3..2b7c55f 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -2243,8 +2243,16 @@ addvars(Estate state, Wordcode pc, int addflags)
 		state->pc = opc;
 		return;
 	    }
-	    if (isset(GLOBASSIGN) || !isstr)
+	    if (!isstr || (isset(GLOBASSIGN) &&
+			   haswilds((char *)getdata(firstnode(vl))))) {
 		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))
+		    unsetparam(name);
+	    }
 	    if (errflag) {
 		state->pc = opc;
 		return;
--- >8 --- snip --- >8 ---


All tests (except X02zlevi some of the time) still pass with either of
the above, but there may be hidden consequences of each.

Re: Inconsistency of GLOB_ASSIGN

[Peter Stephenson]

On Thu, 27 Nov 2014 15:00:36 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Nov 27,  1:38pm, Bart Schaefer wrote:
> }
> } On Nov 27,  8:43pm, Peter Stephenson wrote:
> } }
> } } It would be neater always to do an array assignment, in fact, but
> } } the traditional behaviour is that if there was only one result the
> } } assignment is scalar
> } 
> } That's too bad, because making it always be an array assignment is a
> } lot simpler than figuring out whether the glob expanded to something.
> 
> So in actuality, not too much different.  Below are two patches, pick
> one ... both need the haswilds() test to preserve the behavior of
> assigning a non-glob-string-that-looks-like-math to an integer/float.

Given there not much difference, it would probably be better to stick
with the one that does scalar assignment in the case of a single entry.
I've no idea if anyone is still using GLOB_ASSIGN, but they're not
likely to be complaining about efficiency, and if they always want an
array assignment there's an easy way of getting that.

pws

Re: Inconsistency of GLOB_ASSIGN

[Bart Schaefer]

On Nov 28,  9:34am, Peter Stephenson wrote:
}
} Given there not much difference, it would probably be better to stick
} with the one that does scalar assignment in the case of a single entry.

OK, here's a test.  Is the floating point output of "typeset -p"
consistent, or are there going to be platform differences that require
a different parse?  (I stuck with a power of 2 to avoid rounding.)


diff --git a/Test/A06assign.ztst b/Test/A06assign.ztst
index 9a0a4f0..3c9ea08 100644
--- a/Test/A06assign.ztst
+++ b/Test/A06assign.ztst
@@ -1,5 +1,10 @@
 # Tests of parameter assignments
 
+%prep
+  mkdir assign.tmp && cd assign.tmp
+
+  touch tmpfile1 tmpfile2
+
 %test
 
  typeset -A assoc
@@ -413,3 +418,18 @@
 >world
 >worldliness
 >world
+
+ integer i n x
+ float f
+ setopt globassign
+ i=tmpfile1
+ n=tmp*
+ x=*2
+ f=2+2
+ typeset -p i n x f
+0:GLOB_ASSIGN with numeric types
+>typeset -i i=0
+>typeset -a n
+>n=(tmpfile1 tmpfile2)
+>typeset x=tmpfile2
+>typeset -E f=4.000000000e+00