Zsh Mailing List Archive
Messages sorted by:
Reverse Date,
Date,
Thread,
Author
Re: LOCAL_VARS option ?
On Thu, 19 Jan 2017 21:01:41 -0800
Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
> Perhaps rather than a setopt, this could be a property of functions --
> something like "typeset -fT" so limited to the single function to which
> it is applied. That could be appropriate either for a generic "used
> but not declared local" warning or the "implicit local var" feature,
> though I would still have reservations about the latter.
Here's a first go at the warning option, with "functions -W" to turn it on
in the same fashion as "functions -T". The option is called
WARN_NESTED_VAR for now. If we want a pair of options like -t / -T
then the option will need to change as autoload -w is for compiled
files (i.e. wordcode).
The main drawback over WARN_CREATE_GLOBAL is that "typeset -g" doesn't
mark the variable for future assignments; it's still in a global scope
so still gets a warning next time. I think that's correct behaviour
(it's certainly incredibly fiddly and bug-prone to do anything about).
But this makes it impossible to suppress the warning for a (( ... )),
though "typeset -g var=$(( ... ))" works.
pws
diff --git a/Completion/compinit b/Completion/compinit
index cc663cb..eb88a9f 100644
--- a/Completion/compinit
+++ b/Completion/compinit
@@ -157,6 +157,7 @@ _comp_options=(
NO_posixidentifiers
NO_shwordsplit
NO_shglob
+ NO_warnnestedvar
NO_warncreateglobal
)
diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index 3a3130a..0a9021c 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -147,7 +147,7 @@ ifnzman(noderef(Aliasing)).
findex(autoload)
cindex(functions, autoloading)
cindex(autoloading functions)
-item(tt(autoload) [ {tt(PLUS())|tt(-)}tt(RTUXdkmrtz) ] [ tt(-w) ] [ var(name) ... ])(
+item(tt(autoload) [ {tt(PLUS())|tt(-)}tt(RTUXdkmrtWz) ] [ tt(-w) ] [ var(name) ... ])(
vindex(fpath, searching)
See the section `Autoloading Functions' in ifzman(zmanref(zshmisc))\
ifnzman(noderef(Functions)) for full details. The tt(fpath) parameter
@@ -836,19 +836,24 @@ Equivalent to tt(typeset -E), except that options irrelevant to floating
point numbers are not permitted.
)
findex(functions)
-xitem(tt(functions) [ {tt(PLUS())|tt(-)}tt(UkmtTuz) ] [ tt(-x) var(num) ] [ var(name) ... ])
+xitem(tt(functions) [ {tt(PLUS())|tt(-)}tt(UkmtTuWz) ] [ tt(-x) var(num) ] [ var(name) ... ])
xitem(tt(functions -M) var(mathfn) [ var(min) [ var(max) [ var(shellfn) ] ] ])
xitem(tt(functions -M) [ tt(-m) var(pattern) ... ])
item(tt(functions +M) [ tt(-m) ] var(mathfn) ... )(
-Equivalent to tt(typeset -f), with the exception of the tt(-x) and
-tt(-M) options. For tt(functions -u) and tt(functions -U), see
-tt(autoload), which provides additional options.
+Equivalent to tt(typeset -f), with the exception of the tt(-x),
+tt(-M) and tt(-W) options. For tt(functions -u) and tt(functions -U),
+see tt(autoload), which provides additional options.
The tt(-x) option indicates that any functions output will have
each leading tab for indentation, added by the shell to show syntactic
structure, expanded to the given number var(num) of spaces. var(num)
can also be 0 to suppress all indentation.
+The tt(-W) option turns on the option tt(WARN_NESTED_VAR) for the named
+function or functions only. The option is turned off at the start of
+nested functions (apart from anonoymous functions) unless the called
+function also has the tt(-W) attribute.
+
Use of the tt(-M) option may not be combined with any of the options
handled by tt(typeset -f).
diff --git a/Doc/Zsh/options.yo b/Doc/Zsh/options.yo
index 434b710..aa62748 100644
--- a/Doc/Zsh/options.yo
+++ b/Doc/Zsh/options.yo
@@ -768,6 +768,37 @@ global from within a function using tt(typeset -g) do not cause a warning.
Note that there is no warning when a local parameter is assigned to in
a nested function, which may also indicate an error.
)
+pindex(WARN_NESTED_VAR)
+pindex(NO_WARN_NESTED_VAR)
+pindex(WARNNESTEDVAR)
+pindex(NO_WARNNESTEDVAR)
+cindex(parameters, warning when setting in enclosing scope)
+item(tt(WARN_NESTED_VAR))(
+Print a warning message when an existing parameter from an
+enclosing function scope, or global, is set in a function
+by an assignment or in math context. Assignment to shell
+special parameters does not cause a warning. This is the companion
+to tt(WARN_CREATE_GLOBAL) as in this case the warning is only
+printed when a parameter is em(not) created. Where possible,
+use of tt(typeset -g) to set the parameter suppresses the error,
+but note that this needs to be used every time the parameter is set.
+To restrict the effect of this option to a single function scope,
+use `tt(functions -W)'.
+
+For example, the following code produces a warning for the assignment
+inside the function tt(nested) as that overrides the value within
+tt(toplevel)
+
+example(toplevel+LPAR()RPAR() {
+ local foo="in fn"
+ nested
+}
+nested+LPAR()RPAR() {
+ foo="in nested"
+}
+setopt warn_nested_var
+toplevel)
+)
enditem()
subsect(History)
diff --git a/Src/builtin.c b/Src/builtin.c
index 7a04a79..2fb1a70 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -46,7 +46,7 @@ static struct builtin builtins[] =
BUILTIN(".", BINF_PSPECIAL, bin_dot, 1, -1, 0, NULL, NULL),
BUILTIN(":", BINF_PSPECIAL, bin_true, 0, -1, 0, NULL, NULL),
BUILTIN("alias", BINF_MAGICEQUALS | BINF_PLUSOPTS, bin_alias, 0, -1, 0, "Lgmrs", NULL),
- BUILTIN("autoload", BINF_PLUSOPTS, bin_functions, 0, -1, 0, "dmktrRTUwXz", "u"),
+ BUILTIN("autoload", BINF_PLUSOPTS, bin_functions, 0, -1, 0, "dmktrRTUwWXz", "u"),
BUILTIN("bg", 0, bin_fg, 0, -1, BIN_BG, NULL, NULL),
BUILTIN("break", BINF_PSPECIAL, bin_break, 0, 1, BIN_BREAK, NULL, NULL),
BUILTIN("bye", 0, bin_break, 0, 1, BIN_EXIT, NULL, NULL),
@@ -72,7 +72,7 @@ static struct builtin builtins[] =
BUILTIN("fc", 0, bin_fc, 0, -1, BIN_FC, "aAdDe:EfiIlLmnpPrRt:W", NULL),
BUILTIN("fg", 0, bin_fg, 0, -1, BIN_FG, NULL, NULL),
BUILTIN("float", BINF_PLUSOPTS | BINF_MAGICEQUALS | BINF_PSPECIAL | BINF_ASSIGN, (HandlerFunc)bin_typeset, 0, -1, 0, "E:%F:%HL:%R:%Z:%ghlprtux", "E"),
- BUILTIN("functions", BINF_PLUSOPTS, bin_functions, 0, -1, 0, "kmMtTuUx:z", NULL),
+ BUILTIN("functions", BINF_PLUSOPTS, bin_functions, 0, -1, 0, "kmMtTuUWx:z", NULL),
BUILTIN("getln", 0, bin_read, 0, -1, 0, "ecnAlE", "zr"),
BUILTIN("getopts", 0, bin_getopts, 2, -1, 0, NULL, NULL),
BUILTIN("hash", BINF_MAGICEQUALS, bin_hash, 0, -1, 0, "Ldfmrv", NULL),
@@ -796,8 +796,8 @@ set_pwd_env(void)
unsetparam_pm(pm, 0, 1);
}
- setsparam("PWD", ztrdup(pwd));
- setsparam("OLDPWD", ztrdup(oldpwd));
+ assignsparam("PWD", ztrdup(pwd), 0);
+ assignsparam("OLDPWD", ztrdup(oldpwd), 0);
pm = (Param) paramtab->getnode(paramtab, "PWD");
if (!(pm->node.flags & PM_EXPORTED))
@@ -3068,6 +3068,10 @@ bin_functions(char *name, char **argv, Options ops, int func)
on |= PM_TAGGED_LOCAL;
else if (OPT_PLUS(ops,'T'))
off |= PM_TAGGED_LOCAL;
+ if (OPT_MINUS(ops,'W'))
+ on |= PM_WARNNESTED;
+ else if (OPT_PLUS(ops,'W'))
+ off |= PM_WARNNESTED;
roff = off;
if (OPT_MINUS(ops,'z')) {
on |= PM_ZSHSTORED;
diff --git a/Src/exec.c b/Src/exec.c
index 6c82643..8f4969f 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -2379,9 +2379,7 @@ addvars(Estate state, Wordcode pc, int addflags)
* to be restored after the command, since then the assignment
* is implicitly scoped.
*/
- flags = (!(addflags & ADDVAR_RESTORE) &&
- locallevel > forklevel && isset(WARNCREATEGLOBAL)) ?
- ASSPM_WARN_CREATE : 0;
+ flags = !(addflags & ADDVAR_RESTORE) ? ASSPM_WARN : 0;
xtr = isset(XTRACE);
if (xtr) {
printprompt4();
@@ -5431,6 +5429,14 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
else
opts[XTRACE] = 0;
}
+ if (flags & PM_WARNNESTED)
+ opts[WARNNESTEDVAR] = 1;
+ else if (oflags & PM_WARNNESTED) {
+ if (shfunc->node.nam == ANONYMOUS_FUNCTION_NAME)
+ flags |= PM_WARNNESTED;
+ else
+ opts[WARNNESTEDVAR] = 0;
+ }
ooflags = oflags;
/*
* oflags is static, because we compare it on the next recursive
@@ -5549,6 +5555,7 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
opts[PRINTEXITVALUE] = saveopts[PRINTEXITVALUE];
opts[LOCALOPTIONS] = saveopts[LOCALOPTIONS];
opts[LOCALLOOPS] = saveopts[LOCALLOOPS];
+ opts[WARNNESTEDVAR] = saveopts[WARNNESTEDVAR];
}
if (opts[LOCALLOOPS]) {
diff --git a/Src/options.c b/Src/options.c
index 4729ba5..e0b67d2 100644
--- a/Src/options.c
+++ b/Src/options.c
@@ -258,6 +258,7 @@ static struct optname optns[] = {
{{NULL, "verbose", 0}, VERBOSE},
{{NULL, "vi", 0}, VIMODE},
{{NULL, "warncreateglobal", OPT_EMULATE}, WARNCREATEGLOBAL},
+{{NULL, "warnnestedvar", OPT_EMULATE}, WARNNESTEDVAR},
{{NULL, "xtrace", 0}, XTRACE},
{{NULL, "zle", OPT_SPECIAL}, USEZLE},
{{NULL, "braceexpand", OPT_ALIAS}, /* ksh/bash */ -IGNOREBRACES},
diff --git a/Src/params.c b/Src/params.c
index d490466..ebdd252 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -2849,20 +2849,47 @@ gethkparam(char *s)
return NULL;
}
+/*
+ * Function behind WARNCREATEGLOBAL and WARNNESTEDVAR option.
+ *
+ * For WARNNESTEDVAR:
+ * Called when the variable is created.
+ * Apply heuristics to see if this variable was just created
+ * globally but in a local context.
+ *
+ * For WARNNESTEDVAR:
+ * Called when the variable already exists and is set.
+ * Apply heuristics to see if this variable is setting
+ * a variable that was created in a less nested function
+ * or globally.
+ */
+
/**/
static void
-check_warn_create(Param pm, const char *pmtype)
+check_warn_pm(Param pm, const char *pmtype, int created)
{
Funcstack i;
- if (pm->level != 0 || (pm->node.flags & PM_SPECIAL))
+ if (created && isset(WARNCREATEGLOBAL)) {
+ if (locallevel <= forklevel || pm->level != 0)
+ return;
+ } else if (!created && isset(WARNNESTEDVAR)) {
+ if (pm->level >= locallevel)
+ return;
+ } else
+ return;
+
+ if (pm->node.flags & PM_SPECIAL)
return;
for (i = funcstack; i; i = i->prev) {
if (i->tp == FS_FUNC) {
+ char *msg;
DPUTS(!i->name, "funcstack entry with no name");
- zwarn("%s parameter %s created globally in function %s",
- pmtype, pm->node.nam, i->name);
+ msg = created ?
+ "%s parameter %s created globally in function %s" :
+ "%s parameter %s set in enclosing scope in function %s";
+ zwarn(msg, pmtype, pm->node.nam, i->name);
break;
}
}
@@ -2923,8 +2950,8 @@ assignsparam(char *s, char *val, int flags)
/* errflag |= ERRFLAG_ERROR; */
return NULL;
}
- if (flags & ASSPM_WARN_CREATE)
- check_warn_create(v->pm, "scalar");
+ if (flags & ASSPM_WARN)
+ check_warn_pm(v->pm, "scalar", flags & ASSPM_WARN_CREATE);
if (flags & ASSPM_AUGMENT) {
if (v->start == 0 && v->end == -1) {
switch (PM_TYPE(v->pm->node.flags)) {
@@ -3005,9 +3032,7 @@ assignsparam(char *s, char *val, int flags)
mod_export Param
setsparam(char *s, char *val)
{
- return assignsparam(
- s, val, isset(WARNCREATEGLOBAL) && locallevel > forklevel ?
- ASSPM_WARN_CREATE : 0);
+ return assignsparam(s, val, ASSPM_WARN);
}
/**/
@@ -3074,8 +3099,8 @@ assignaparam(char *s, char **val, int flags)
return NULL;
}
- if (flags & ASSPM_WARN_CREATE)
- check_warn_create(v->pm, "array");
+ if (flags & ASSPM_WARN)
+ check_warn_pm(v->pm, "array", flags & ASSPM_WARN_CREATE);
if (flags & ASSPM_AUGMENT) {
if (v->start == 0 && v->end == -1) {
if (PM_TYPE(v->pm->node.flags) & PM_ARRAY) {
@@ -3103,9 +3128,7 @@ assignaparam(char *s, char **val, int flags)
mod_export Param
setaparam(char *s, char **aval)
{
- return assignaparam(
- s, aval, isset(WARNCREATEGLOBAL) && locallevel > forklevel ?
- ASSPM_WARN_CREATE : 0);
+ return assignaparam(s, aval, ASSPM_WARN);
}
/**/
@@ -3134,7 +3157,7 @@ sethparam(char *s, char **val)
queue_signals();
if (!(v = fetchvalue(&vbuf, &s, 1, SCANPM_ASSIGNING))) {
createparam(t, PM_HASHED);
- checkcreate = isset(WARNCREATEGLOBAL) && locallevel > forklevel;
+ checkcreate = 1;
} else if (!(PM_TYPE(v->pm->node.flags) & PM_HASHED) &&
!(v->pm->node.flags & PM_SPECIAL)) {
unsetparam(t);
@@ -3148,8 +3171,7 @@ sethparam(char *s, char **val)
/* errflag |= ERRFLAG_ERROR; */
return NULL;
}
- if (checkcreate)
- check_warn_create(v->pm, "associative array");
+ check_warn_pm(v->pm, "associative array", checkcreate);
setarrvalue(v, val);
unqueue_signals();
return v->pm;
@@ -3214,8 +3236,9 @@ setnparam(char *s, mnumber val)
unqueue_signals();
return NULL;
}
- if (!was_unset && isset(WARNCREATEGLOBAL) && locallevel > forklevel)
- check_warn_create(v->pm, "numeric");
+ check_warn_pm(v->pm, "numeric", !was_unset);
+ } else {
+ check_warn_pm(v->pm, "numeric", 0);
}
setnumvalue(v, val);
unqueue_signals();
@@ -3250,10 +3273,7 @@ setiparam_no_convert(char *s, zlong val)
*/
char buf[BDIGBUFSIZE];
convbase(buf, val, 10);
- return assignsparam(
- s, ztrdup(buf),
- isset(WARNCREATEGLOBAL) && locallevel > forklevel ?
- ASSPM_WARN_CREATE : 0);
+ return assignsparam(s, ztrdup(buf), ASSPM_WARN);
}
/* Unset a parameter */
diff --git a/Src/zsh.h b/Src/zsh.h
index 7d18333..d022260 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -1815,6 +1815,7 @@ struct tieddata {
#define PM_HIDE (1<<14) /* Special behaviour hidden by local */
#define PM_CUR_FPATH (1<<14) /* (function): can use $fpath with filename */
#define PM_HIDEVAL (1<<15) /* Value not shown in `typeset' commands */
+#define PM_WARNNESTED (1<<15) /* (function): non-recursive WARNNESTEDVAR */
#define PM_TIED (1<<16) /* array tied to colon-path or v.v. */
#define PM_TAGGED_LOCAL (1<<16) /* (function): non-recursive PM_TAGGED */
@@ -2016,9 +2017,15 @@ struct paramdef {
* Flags for assignsparam and assignaparam.
*/
enum {
+ /* Add to rather than override value */
ASSPM_AUGMENT = 1 << 0,
+ /* Test for warning if creating global variable in function */
ASSPM_WARN_CREATE = 1 << 1,
- ASSPM_ENV_IMPORT = 1 << 2
+ /* Test for warning if using nested variable in function */
+ ASSPM_WARN_NESTED = 1 << 2,
+ ASSPM_WARN = (ASSPM_WARN_CREATE|ASSPM_WARN_NESTED),
+ /* Import from environment, so exercise care evaluating value */
+ ASSPM_ENV_IMPORT = 1 << 3,
};
/* node for named directory hash table (nameddirtab) */
@@ -2400,6 +2407,7 @@ enum {
VERBOSE,
VIMODE,
WARNCREATEGLOBAL,
+ WARNNESTEDVAR,
XTRACE,
USEZLE,
DVORAK,
diff --git a/Test/E01options.ztst b/Test/E01options.ztst
index 45df9f5..bcd89f7 100644
--- a/Test/E01options.ztst
+++ b/Test/E01options.ztst
@@ -1118,7 +1118,8 @@
integer foo6=9
(( foo6=10 ))
}
- fn
+ # don't pollute the test environment with the variables...
+ (fn)
0:WARN_CREATE_GLOBAL option
?fn:3: scalar parameter foo1 created globally in function fn
?fn:5: scalar parameter foo1 created globally in function fn
@@ -1133,6 +1134,60 @@
fn
0:WARN_CREATE_GLOBAL negative cases
+ (
+ foo1=global1 foo2=global2 foo3=global3 foo4=global4
+ integer foo5=5
+ fn_wnv() {
+ # warns
+ foo1=bar1
+ # doesn't warn
+ local foo2=bar3
+ unset foo2
+ # still doesn't warn
+ foo2=bar4
+ # doesn't warn
+ typeset -g foo3=bar5
+ # warns
+ foo3=bar6
+ fn2() {
+ # warns if global option, not attribute
+ foo3=bar6
+ }
+ fn2
+ # doesn't warn
+ foo4=bar7 =true
+ # warns
+ (( foo5=8 ))
+ integer foo6=9
+ # doesn't warn
+ (( foo6=10 ))
+ }
+ print option off >&2
+ fn_wnv
+ print option on >&2
+ setopt warnnestedvar
+ fn_wnv
+ unsetopt warnnestedvar
+ print function attribute on >&2
+ functions -W fn_wnv
+ fn_wnv
+ print all off again >&2
+ functions +W fn_wnv
+ fn_wnv
+ )
+0:WARN_NESTED_VAR option
+?option off
+?option on
+?fn_wnv:2: scalar parameter foo1 set in enclosing scope in function fn_wnv
+?fn_wnv:11: scalar parameter foo3 set in enclosing scope in function fn_wnv
+?fn2:2: scalar parameter foo3 set in enclosing scope in function fn2
+?fn_wnv:20: numeric parameter foo5 set in enclosing scope in function fn_wnv
+?function attribute on
+?fn_wnv:2: scalar parameter foo1 set in enclosing scope in function fn_wnv
+?fn_wnv:11: scalar parameter foo3 set in enclosing scope in function fn_wnv
+?fn_wnv:20: numeric parameter foo5 set in enclosing scope in function fn_wnv
+?all off again
+
# This really just tests if XTRACE is egregiously broken.
# To test it properly would need a full set of its own.
fn() { print message; }
Messages sorted by:
Reverse Date,
Date,
Thread,
Author