PATCH: "typeset -m" plays havoc

PATCH: "typeset -m" plays havoc

[Bart Schaefer]

Don't try this:

zsh% splat() { typeset -m \* }
zsh% splat

The result is that all non-readonly/non-special parameters get stomped on,
which leaves zsh in a pretty sorry state.

The behavior up to 3.1.5 or so was that `-m' implied the equivalent of `-g',
so I think the following is the most expedient patch.  Peter?

Index: Src/builtin.c
===================================================================
@@ -1942,6 +1942,7 @@
 
     /* With the -m option, treat arguments as glob patterns */
     if (ops['m']) {
+	on &= ~PM_LOCAL;
 	while ((asg = getasg(*argv++))) {
 	    LinkList pmlist = newlinklist();
 	    LinkNode pmnode;

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

RE: PATCH: "typeset -m" plays havoc

[Andrej Borsenkow]

> Don't try this:
>
> zsh% splat() { typeset -m \* }
> zsh% splat
>
> The result is that all non-readonly/non-special parameters get stomped on,
> which leaves zsh in a pretty sorry state.
>
> The behavior up to 3.1.5 or so was that `-m' implied the equivalent of `-g',
> so I think the following is the most expedient patch.  Peter?
>


What happens in case of

splat() { typeset -F -m \* }

??

Currently it creates local floating point parameters and then breaks on first
special one (in may case it is cdpath). With this patch, won't it silently
change type of global parameters?

Also, consider:

bor@itsrm2% splat () {typeset cdpath}
bor@itsrm2% splat

If I can believe manual:

     For each remaining NAME that refers to a parameter that is set, the
     name and value of the parameter are printed in the form of an
     assignment.  Nothing is printed for newly-created parameters, or
     if any attribute flags listed below are given.  Using `+' instead
     of minus to introduce an attribute turns it off.

In our case cdpath is definitely set; but what we get is creation of local
cdpath.

The suggested patch seems to take care of the both. It is on top of Bart's.
Peter?

Index: Src/builtin.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/builtin.c,v
retrieving revision 1.32
diff -u -r1.32 builtin.c
--- Src/builtin.c       2000/09/14 15:55:00     1.32
+++ Src/builtin.c       2000/09/14 16:54:31
@@ -1800,7 +1800,7 @@
     Asgment asg;
     Patprog pprog;
     char *optstr = TYPESET_OPTSTR;
-    int on = 0, off = 0, roff, bit = PM_ARRAY;
+    int on = 0, off = 0, ron, roff, bit = PM_ARRAY;
     int i;
     int returnval = 0, printflags = 0;

@@ -1816,6 +1816,7 @@
            on |= bit;
        else if (ops[STOUC(*optstr)] == 2)
            off |= bit;
+    ron = on;
     roff = off;

     /* Sanity checks on the options.  Remove conficting options. */
@@ -1858,7 +1859,8 @@
        return 0;
     }

-    if ((!ops['g'] && !ops['x']) || ops['g'] == 2 || *name == 'l' ||
+    if ((!ops['g'] && !ops['x'] && (ron || roff))
+       || ops['g'] == 2 || *name == 'l' ||
        !isset(GLOBALEXPORT))
        on |= PM_LOCAL;

@@ -1942,7 +1944,6 @@

     /* With the -m option, treat arguments as glob patterns */
     if (ops['m']) {
-       on &= ~PM_LOCAL;
        while ((asg = getasg(*argv++))) {
            LinkList pmlist = newlinklist();
            LinkNode pmnode;

Re: PATCH: "typeset -m" plays havoc

[Bart Schaefer]

On Sep 14,  8:55pm, Andrej Borsenkow wrote:
} Subject: RE: PATCH: "typeset -m" plays havoc
}
} What happens in case of
} 
} splat() { typeset -F -m \* }

Depending on what stands in for -F, it dumps core.  I just noticed that
myself.

} bor@itsrm2% splat () {typeset cdpath}
} bor@itsrm2% splat
} 
} If I can believe manual:
} 
}      For each remaining NAME that refers to a parameter that is set, the
}      name and value of the parameter are printed
} 
} In our case cdpath is definitely set; but what we get is creation of local
} cdpath.

Which is what must happen, otherwise you can never create local parameters
with the same names as global ones and the whole point of "local" is lost.
Try a similar function in ksh or bash.

} The suggested patch seems to take care of the both. It is on top of Bart's.
} Peter?

No, please don't apply/commit 12806.

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

Re: PATCH: "typeset -m" plays havoc

[Bart Schaefer]

On Sep 14,  5:35pm, Bart Schaefer wrote:
> Subject: Re: PATCH: "typeset -m" plays havoc
> On Sep 14,  8:55pm, Andrej Borsenkow wrote:
> } Subject: RE: PATCH: "typeset -m" plays havoc
> }
> } What happens in case of
> } 
> } splat() { typeset -F -m \* }
> 
> Depending on what stands in for -F, it dumps core.

Which has nothing to do with my patch, by the way.

Bad Things happen in every version of zsh back to 2.4 if you specify other
type-declaration flags with -m.  How bad, varies from "mostly harmless" to
"complete disaster" depending on which version and which flags, with the
more recent versions having the greater potential for flame-out.

Re: PATCH: "typeset -m" plays havoc

[Bart Schaefer]

On Sep 14, 11:20am, Bart Schaefer wrote:
}
} Bad Things happen in every version of zsh back to 2.4 if you specify other
} type-declaration flags with -m.

Staring at this for a bit convinced me that, at least in recent versions
of zsh, this can't really have anything to do with `-m'.  What it means
is that zsh is not robust in the face of certain "nonsense" typesets of
variables that it uses -- any bad thing you can make happen with `-m' it
should be possible to make happen without it.

Here's an example:

    erase_PS1() { typeset PS1 prompt PROMPT }

Run that function, and your prompt will disappear, even though all it does
(supposedly) is declare local parameters with the same names.  (This bug
can't happen in older versions where those parameters are "special".)  The
bug doesn't happen if you use `typeset PROMPT prompt PS1', but with any
other ordering you erase all three by making any two or more of them local.

Similar things can be caused to happen to any of the other non-special,
scalar variables that are "tied" (PS2/PROMPT2, histchars/HISTCHARS, etc.)
as long as you make them local in the right (er, wrong?) order.

Another example:

    flush_history() { typeset HISTSIZE }

In fact, the above examples cover all the bad behavior that caused me to
notice the problem with `typeset -m' in the first place.

Other crashes happen when changing the type of assorted variables that zsh
uses.  Here's a good one:

    PS1='%m%# '
    HOST=()

If HOST is an empty array and any prompt-expansion string uses "%m", zsh
will dump core.  This is new as of zsh-workers/12247.

Next there appears to be a bad interaction between auto-zmodloaded special
parameters and builtin.c:typeset_single().  Normally typeset_single() will
choke with "can't change type of a special parameter", but that appears to
be tested too early -- the parameter may come into existence because of
autoloading between the time the PM_SPECIAL flag is tested and the time
the change-of-type is attempted.  This leads to a core dump.  This does
look like a bug strictly with `-m', having to do with the way that the
parameter hash table is searched when doing the pattern match:  The auto-
loaded parameters are in the table but have none of their flags set yet.

That's all I have time to hunt around for right now, but it does look as
if it's not necessary to force `typeset -m' to work on globals.  AFAICT,
the reason it worked this way in previous versions was because of the
parameter table search:  Since you can't possibly refer to a non-existing
parameter with `typeset -m', you must therefore be referring to the one
that already exists.  Now that we have `+g', though, there's no reason
one can't say "find this global parameter and force it to be local".  So
I'll send a revised patch (a subset of Andrej's 12806, really) as soon as
I have time to revise the doc slightly to match.

That patch probably *won't* address the list of problems above, though ...

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

RE: PATCH: "typeset -m" plays havoc

[Andrej Borsenkow]

>     erase_PS1() { typeset PS1 prompt PROMPT }

Hmm ... in case of prompts what (very probably) happens:

- for the first parameter typeset_single() calls copyparam() to save old
value; this creates new parameter and copies old *string* value. Then it sets
value to "" creating new empty parameter - that actually sets variable
"prompt" to empty string.
- for the second and third parameters exactly the same happens ... but now the
saved value is already empty string (all three actually refer to the same
variable "prompt").

When functions exits it restores values - I think, it depends on order of
paramtable scan which of three it hits. If it hits the one with empty value
last, prompt gets reset to empty value.

>     flush_history() { typeset HISTSIZE }

In case of HISTSIZE it preserves the value, of course, but in typeset_single()
it sets value of new parameter that has side effect of flushing history. So
far it is correct :-) assuming that set/get functions do what they are
expected to.

>
> That's all I have time to hunt around for right now, but it does look as
> if it's not necessary to force `typeset -m' to work on globals.  AFAICT,
> the reason it worked this way in previous versions was because of the
> parameter table search:  Since you can't possibly refer to a non-existing
> parameter with `typeset -m', you must therefore be referring to the one
> that already exists.  Now that we have `+g', though, there's no reason
> one can't say "find this global parameter and force it to be local".  So
> I'll send a revised patch (a subset of Andrej's 12806, really) as soon as
> I have time to revise the doc slightly to match.
>

I do not quite understand, what typeset -m is supposed to do.

1. If used alone, typeset -m is expected to just print parameters. It is
currently broken, because typeset_single() will print only if there are no
flags to set, and bin_typeset() will (almost) always set at least PM_LOCAL.
Patch in 12806 was wrong of course, but we should prevent setting PM_LOCAL in
this case.

2. What I do not understand, what typeset -m es expected do if any flag is
present? It may be interpreted as "print parameters that match and have this
flag set" or "set flags for matching parameters". In the former case
typeset_single() is broken. The semantic of latter is unclear (wrt
global/local *) but in any case it should not create new parameters. But,
unfortunately, this happens currently for all I can tell.

*) I mean, allowing it to change global parameters makes things really
confusing. Typeset is expected normally to work for local parameters only.


-andrej

Re: PATCH: "typeset -m" plays havoc

[Bart Schaefer]

On Sep 15,  9:36pm, Andrej Borsenkow wrote:
> 
> I do not quite understand, what typeset -m is supposed to do.

Historically (e.g., zsh 2.4, which I happen to have found on an old disk
and recompiled recently):

-  `typeset -m' always works on existing parameters that are "visible" in
   the current scope, regardless of the scope in which they were set or
   will be unset/restored.
-  `typeset -m ...' with no other options prints those parameters.
-  with other options, it changes the associated attributes of all the
   existing, matching parameters.

That last is a lot of rope with which to hang yourself, and zsh was never
thoroughly tested to be sure you couldn't take the whole shell with you.

What I propose is to make the rope slightly longer, and actually cause `+g'
to mean something:

	typeset +g -m foo

would mean to create a local parameter named foo if an only if there is
already a parameter named foo visible in the current scope.  (Without the
`-m', `+g' already unconditionally creates a local parameter.)

This would be useful mostly as a shorthand, e.g. (assuming the other bugs
with localizing certain parameters get fixed) the prompt preview code may
want to do something like:

	typeset +g -h -m '(#i)(prompt|ps)*'

> *) I mean, allowing it to change global parameters makes things really
> confusing. Typeset is expected normally to work for local parameters only.

My position is that:

-  There's historical precedent for this behavior.
-  `typeset -m' is unique to zsh, so it's not a conflict with semantics
   for sh or ksh scripts.
-  You can't otherwise refer to a parameter by pattern, so it shouldn't
   be too difficult to realize you're doing something unusual.
-  It's useful to be able to dump the values of selected parameters in
   the current scope without jumping through the sorts of hoops that
   e.g. Util/reporter does.

On the latter note, I'd like to change `typeset +m' to mean something akin
to `typeset +' in that it would dump only the names and type information
rather than the values.  (`typeset + name' dumps the name and value.)  But
that's a more significant change, so I probably won't, just yet.

PATCH: "typeset +m ..." and "typeset +g -m ..."

[Bart Schaefer]

On Sep 15, 11:46am, Bart Schaefer wrote:
} Subject: Re: PATCH: "typeset -m" plays havoc
}
} What I propose is to ... actually cause `+g' to mean something:
} 
} 	typeset +g -m foo
} 
} would mean to create a local parameter named foo if an only if there is
} already a parameter named foo visible in the current scope.
} 
} [And] ... I'd like to change `typeset +m' to mean something akin
} to `typeset +' in that it would dump only the names and type information
} rather than the values.

The latter was easier than I expected, so here's the patch for both.  I
made a couple of other minor edits to the typeset doc along the way, too.

All that doc to describe 11 lines of code ...

Index: Doc/Zsh/builtins.yo
===================================================================
@@ -1042,9 +1042,9 @@
 
 For each remaining var(name) that refers to a parameter that is set, the
 name and value of the parameter are printed in the form of an assignment.
-Nothing is printed for newly-created parameters, or if any attribute flags
-listed below are given.  Using `tt(PLUS())' instead of minus to introduce
-an attribute turns it off.
+Nothing is printed for newly-created parameters, or when any attribute
+flags listed below are given along with the var(name).  Using `tt(PLUS())'
+instead of minus to introduce an attribute turns it off.
 
 If the tt(-T) option is given, exactly two (or zero) var(name)
 arguments must be present.  They represent a scalar and an array (in
@@ -1058,7 +1058,7 @@
 type of one of them with another tt(typeset) command; tt(+T) does not
 work, assigning an array to var(SCALAR) is an error, and assigning a
 scalar to var(array) sets it to be a single-element array.  Note that
-both tt(typeset -xT ...) and tt(export -T ...) work, but only the
+both `tt(typeset -xT ...)' and `tt(export -T ...)' work, but only the
 scalar will be marked for export.
 
 The tt(-g) (global) flag is treated specially: it means that any
@@ -1067,20 +1067,32 @@
 will apply to any existing parameter (even if unset) from an enclosing
 function.  This flag does not affect the parameter after creation, hence it
 has no effect when listing existing parameters, nor does the flag tt(+g)
-have any effect.
+have any effect except in combination with tt(-m) (see below).
 
 If no var(name) is present, the names and values of all parameters are
 printed.  In this case the attribute flags restrict the display to only
 those parameters that have the specified attributes, and using `tt(PLUS())'
 rather than `tt(-)' to introduce the flag suppresses printing of the values
-of parameters when there is no parameter name.  Also, if the option list
-ends with `tt(PLUS())', values will not be printed.  If only the tt(-m)
-flag is given the arguments are taken as patterns (which should be quoted)
-and all parameters (or functions with the tt(-f) flag) with matching names
-are printed.  If no attribute flags and no tt(-m) flag is present, the
-parameter names will be preceded by a list of any attributes (tt(array),
-tt(association), tt(exported), tt(integer), tt(readonly)).
+of parameters when there is no parameter name.  Also, if the last option
+is the word `tt(PLUS())', then names are printed but values are not.
 
+If the tt(-m) flag is given the var(name) arguments are taken as patterns
+(which should be quoted).  With no attribute flags, all parameters (or
+functions with the tt(-f) flag) with matching names are printed.  Note that
+tt(-m) is ignored if no patterns are given.  If the tt(+g) flag is combined
+with tt(-m), a new local parameter is created for every matching parameter
+that is not already local.  Otherwise tt(-m) applies all other flags or
+assignments to the existing parameters.  Except when assignments are made
+with var(name)tt(=)var(value), using tt(+m) forces the matching parameters
+to be printed, even inside a function.
+
+If no attribute flags are given and either no tt(-m) flag is present or
+the tt(+m) form was used, each parameter name printed is preceded by a
+list of the attributes of that parameter (tt(array), tt(association),
+tt(exported), tt(integer), tt(readonly)).  If tt(+m) is used with attribute
+flags, and all those flags are introduced with tt(PLUS()), the matching
+parameter names are printed but their values are not.
+
 The following attribute flags may be specified:
 
 startitem()
@@ -1139,7 +1151,7 @@
 item(tt(-h))(
 Hide: only useful for special parameters (those marked `<S>' in the table in
 ifzman(zmanref(zshparams))\
-ifnzman(noderef(Parameters))\
+ifnzman(noderef(Parameters Set By The Shell))\
 ), and for local parameters with the same name as a special parameter,
 though harmless for others.  A special parameter with this attribute will
 not retain its special effect when made local.  Thus after `tt(typeset -h
Index: Src/builtin.c
===================================================================
@@ -1858,7 +1858,7 @@
 	return 0;
     }
 
-    if ((!ops['g'] && !ops['x']) || ops['g'] == 2 || *name == 'l' ||
+    if (!(ops['g'] || ops['x'] || ops['m']) || ops['g'] == 2 || *name == 'l' ||
 	!isset(GLOBALEXPORT))
 	on |= PM_LOCAL;
 
@@ -1942,7 +1942,11 @@
 
     /* With the -m option, treat arguments as glob patterns */
     if (ops['m']) {
-	on &= ~PM_LOCAL;
+	if (!(on|roff))
+	    printflags |= PRINT_TYPE;
+	if (!on)
+	    printflags |= PRINT_NAMEONLY;
+
 	while ((asg = getasg(*argv++))) {
 	    LinkList pmlist = newlinklist();
 	    LinkNode pmnode;
@@ -1952,6 +1956,11 @@
 		untokenize(asg->name);
 		zwarnnam(name, "bad pattern : %s", argv[-1], 0);
 		returnval = 1;
+		continue;
+	    }
+	    if (ops['m'] == 2 && !asg->value) {
+		scanmatchtable(paramtab, pprog, on|roff, 0,
+			       paramtab->printnode, printflags);
 		continue;
 	    }
 	    /*

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

Parameter aliasing? RE: PATCH: "typeset -m" plays havoc

[Andrej Borsenkow]

>
> >     erase_PS1() { typeset PS1 prompt PROMPT }
>
> Hmm ... in case of prompts what (very probably) happens:
>
> - for the first parameter typeset_single() calls copyparam() to save old
> value; this creates new parameter and copies old *string* value.
> Then it sets
> value to "" creating new empty parameter - that actually sets variable
> "prompt" to empty string.
> - for the second and third parameters exactly the same happens ...
> but now the
> saved value is already empty string (all three actually refer to the same
> variable "prompt").
>

Note, that the same happens in case of three different typeset's as well.

The only clean general way to do it is to introduce real aliases instead of
separate parameters internally referencing the same value. In this case first
typeset would save original value and two others would simply do nothing
(well, they would actually print parameters value. But there is no real way to
avoid it).

One point to decide is: should 'unset alias' remove parameter itself (with all
aliases) or just unset current alias? And if we want user-defined aliases?

-andrej

Re: Parameter aliasing? RE: PATCH: "typeset -m" plays havoc

[Bart Schaefer]

On Sep 18, 12:40pm, Andrej Borsenkow wrote:
} Subject: Parameter aliasing? RE: PATCH: "typeset -m" plays havoc
}
} > >     erase_PS1() { typeset PS1 prompt PROMPT }
} 
} The only clean general way to do it is to introduce real aliases
} instead of separate parameters internally referencing the same value.

How about this as an alternate solution:  Along with the shared pointer
for the value of the "tied" parameters, keep track of the locallevel
whenever any one of the parameters is made local.  When unwinding, only
reset the shared pointer if the tracked locallevel is >= the current
locallevel, and in that case also reset the tracked locallevel to one
less than the current locallevel.  That way, all the "tied" parameters
are reset at most once as each function is unwound.

(I note in passing that parameters created with `typeset -T' become un-
tied when either of them is made local, but special tied parameters like
path/PATH do not.)

} [With aliases] first typeset would save original value and two others
} would simply do nothing (well, they would actually print parameters
} value. But there is no real way to avoid it).

I'm not very happy about this idea, particularly if there's no way to
avoid that side-effect.  If it can't be made to work "as if" there are
separate but "tied" parameters, I'd rather keep hunting for another
solution.  It sounds like it has a lot of potential to interact badly
with the PM_HIDE mechanism, too.

Using `typeset -h' is a workaround for the problem, by the way:

    test_PS1() {
	typeset -h prompt PROMPT
	typeset PS1
    }

As to these issues:

} One point to decide is: should 'unset alias' remove parameter itself
} (with all aliases) or just unset current alias?

There's a bigger question than that:  Which name *is* "the parameter
itself" and which are merely aliases?  Or are none of them really the
parameter?

It can't just unset the alias, because the shell has to behave "as if"
the parameter really were unset; e.g., if you unset PROMPT, zsh should
stop printing a top-level prompt, until such time as you assign to one
of PS1, prompt, or PROMPT.  It can't "forget about" the alias, because
`unset PROMPT; PROMPT=foo' has to retain the side-effect of `PS1=foo'.

It's an incompatible change to unset them all when any of them is unset,
though realistically, probably not a change that anyone would notice.

} And if we want user-defined aliases?

If we're going to go that way, we should implement ksh namerefs instead.
(Except I don't know what happens when you assign to a nameref; does it
assign to the referenced variable, or change the name to which the ref
refers?)

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net