From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13728 invoked from network); 8 Aug 2005 13:56:31 -0000 Received: from news.dotsrc.org (HELO a.mx.sunsite.dk) (130.225.247.88) by ns1.primenet.com.au with SMTP; 8 Aug 2005 13:56:31 -0000 Received: (qmail 25611 invoked from network); 8 Aug 2005 13:56:24 -0000 Received: from sunsite.dk (130.225.247.90) by a.mx.sunsite.dk with SMTP; 8 Aug 2005 13:56:24 -0000 Received: (qmail 15042 invoked by alias); 8 Aug 2005 13:56:22 -0000 Mailing-List: contact zsh-workers-help@sunsite.dk; run by ezmlm Precedence: bulk X-No-Archive: yes X-Seq: 21575 Received: (qmail 15033 invoked from network); 8 Aug 2005 13:56:22 -0000 Received: from news.dotsrc.org (HELO a.mx.sunsite.dk) (130.225.247.88) by sunsite.dk with SMTP; 8 Aug 2005 13:56:22 -0000 Received: (qmail 25368 invoked from network); 8 Aug 2005 13:56:22 -0000 Received: from mailhost1.csr.com (HELO MAILSWEEPER01.csr.com) (81.105.217.43) by a.mx.sunsite.dk with SMTP; 8 Aug 2005 13:56:15 -0000 Received: from exchange03.csr.com (unverified [10.100.137.60]) by MAILSWEEPER01.csr.com (Content Technologies SMTPRS 4.3.12) with ESMTP id for ; Mon, 8 Aug 2005 14:54:06 +0100 Received: from news01.csr.com ([10.103.143.38]) by exchange03.csr.com with Microsoft SMTPSVC(5.0.2195.6713); Mon, 8 Aug 2005 14:55:57 +0100 Received: from news01.csr.com (localhost.localdomain [127.0.0.1]) by news01.csr.com (8.13.1/8.12.11) with ESMTP id j78DuE0T025650 for ; Mon, 8 Aug 2005 14:56:14 +0100 Received: from csr.com (pws@localhost) by news01.csr.com (8.13.1/8.13.1/Submit) with ESMTP id j78DuEkJ025647 for ; Mon, 8 Aug 2005 14:56:14 +0100 Message-Id: <200508081356.j78DuEkJ025647@news01.csr.com> X-Authentication-Warning: news01.csr.com: pws owned process doing -bs To: zsh-workers@sunsite.dk (Zsh hackers list) Subject: PATCH: WARN_CREATE_GLOBAL option Date: Mon, 08 Aug 2005 14:56:13 +0100 From: Peter Stephenson X-OriginalArrivalTime: 08 Aug 2005 13:55:57.0217 (UTC) FILETIME=[E6A1B110:01C59C20] X-Spam-Checker-Version: SpamAssassin 3.0.4 (2005-06-05) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.0.4 This new option prints a warning when a global variable is created by an assignment within a function, which is typically the sign of a parameter that should have been declared local. As the documentation notes, it's possible to mark a parameter as global with typeset -g, which circumvents the error, so it's always possible to fix your code up so that it only gives warnings for undeclared variables. Does anybody (particularly those with limited English) think a different option name would be better? Note the limitation mentioned at the end of the documentation. You don't get an error for this: fn() { setopt warncreateglobal local foo=stick fn2() { foo=bar } fn2 # foo is now bar, error or not? } which may or may not indicate an error. This case is quite hard to test for and fix up, so I've left it alone. This case is handled properly: fn() { local foo foo=pub unset foo foo=bar } with no warning, because in zsh foo=bar recreates the parameter locally. (I mention this explicitly because I forgot it to begin with and had to add an extra test.) I've explicitly turned it off for the completion code for reasons which anyone who turns it back on will immediately note. If someone wants to fix up the warnings, either by making the offending parameter local or by using typeset -g, then we could turn the option on and make the completion system safer. However, as has been noted recently, it's not necessarily immediately obvious which parameters should be local. It's possible I've missed some cases where the warning should or shouldn't be triggered, but this particular part of the parameter code isn't as horrendous as other parts, so it may be OK. Index: Completion/compinit =================================================================== RCS file: /cvsroot/zsh/zsh/Completion/compinit,v retrieving revision 1.13 diff -u -r1.13 compinit --- Completion/compinit 22 Oct 2004 15:52:00 -0000 1.13 +++ Completion/compinit 8 Aug 2005 13:53:50 -0000 @@ -146,6 +146,7 @@ NO_aliases NO_errexit NO_octalzeroes + NO_warncreateglobal ) # And this one should be `eval'ed at the beginning of every entry point Index: Doc/Zsh/options.yo =================================================================== RCS file: /cvsroot/zsh/zsh/Doc/Zsh/options.yo,v retrieving revision 1.40 diff -u -r1.40 options.yo --- Doc/Zsh/options.yo 26 Jul 2005 22:48:43 -0000 1.40 +++ Doc/Zsh/options.yo 8 Aug 2005 13:53:50 -0000 @@ -460,6 +460,16 @@ Treat unset parameters as if they were empty when substituting. Otherwise they are treated as an error. ) +pindex(WARN_CREATE_GLOBAL) +cindex(parameters, warning when created globally) +item(tt(WARN_CREATE_GLOBAL))( +Print a warning message when a global parameter is created in a function +by an assignment. This often indicates that a parameter has not been +declared local when it should have been. Parameters explicitly declared +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. +) enditem() subsect(History) Index: Src/exec.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/exec.c,v retrieving revision 1.90 diff -u -r1.90 exec.c --- Src/exec.c 27 Apr 2005 09:58:45 -0000 1.90 +++ Src/exec.c 8 Aug 2005 13:53:51 -0000 @@ -1649,10 +1649,14 @@ LinkList vl; int xtr, isstr, htok = 0; char **arr, **ptr, *name; + int flags, augment; + Wordcode opc = state->pc; wordcode ac; local_list1(svl); + flags = (locallevel > 0 && isset(WARNCREATEGLOBAL)) ? + ASSPM_WARN_CREATE : 0; xtr = isset(XTRACE); if (xtr) { printprompt4(); @@ -1660,12 +1664,15 @@ } state->pc = pc; while (wc_code(ac = *state->pc++) == WC_ASSIGN) { + int myflags = flags; name = ecgetstr(state, EC_DUPTOK, &htok); if (htok) untokenize(name); + if (WC_ASSIGN_TYPE2(ac) == WC_ASSIGN_INC) + myflags |= ASSPM_AUGMENT; if (xtr) fprintf(xtrerr, - WC_ASSIGN_TYPE2(ac) == WC_ASSIGN_INC ? "%s+=" : "%s=", name); + WC_ASSIGN_TYPE2(ac) == WC_ASSIGN_INC ? "%s+=" : "%s=", name); if ((isstr = (WC_ASSIGN_TYPE(ac) == WC_ASSIGN_SCALAR))) { init_list1(svl, ecgetstr(state, EC_DUPTOK, &htok)); vl = &svl; @@ -1716,12 +1723,10 @@ } allexp = opts[ALLEXPORT]; opts[ALLEXPORT] = 1; - pm = assignsparam(name, val, - WC_ASSIGN_TYPE2(ac) == WC_ASSIGN_INC); + pm = assignsparam(name, val, myflags); opts[ALLEXPORT] = allexp; } else - pm = assignsparam(name, val, - WC_ASSIGN_TYPE2(ac) == WC_ASSIGN_INC); + pm = assignsparam(name, val, myflags); if (errflag) { state->pc = opc; return; @@ -1746,7 +1751,7 @@ } fprintf(xtrerr, ") "); } - assignaparam(name, arr, WC_ASSIGN_TYPE2(ac) == WC_ASSIGN_INC); + assignaparam(name, arr, myflags); if (errflag) { state->pc = opc; return; Index: Src/options.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/options.c,v retrieving revision 1.23 diff -u -r1.23 options.c --- Src/options.c 15 Jul 2005 17:41:50 -0000 1.23 +++ Src/options.c 8 Aug 2005 13:53:51 -0000 @@ -211,6 +211,7 @@ {NULL, "unset", OPT_EMULATE|OPT_BSHELL, UNSET}, {NULL, "verbose", 0, VERBOSE}, {NULL, "vi", 0, VIMODE}, +{NULL, "warncreateglobal", 0, WARNCREATEGLOBAL}, {NULL, "xtrace", 0, XTRACE}, {NULL, "zle", OPT_SPECIAL, USEZLE}, {NULL, "braceexpand", OPT_ALIAS, /* ksh/bash */ -IGNOREBRACES}, Index: Src/params.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/params.c,v retrieving revision 1.99 diff -u -r1.99 params.c --- Src/params.c 24 Jul 2005 05:37:23 -0000 1.99 +++ Src/params.c 8 Aug 2005 13:53:51 -0000 @@ -1990,7 +1990,7 @@ /**/ mod_export Param -assignsparam(char *s, char *val, int augment) +assignsparam(char *s, char *val, int flags) { struct value vbuf; Value v; @@ -2011,12 +2011,14 @@ *ss = '\0'; if (!(v = getvalue(&vbuf, &s, 1))) createparam(t, PM_ARRAY); + else + flags &= ~ASSPM_WARN_CREATE; *ss = '['; v = NULL; } else { if (!(v = getvalue(&vbuf, &s, 1))) createparam(t, PM_SCALAR); - else if ((((v->pm->flags & PM_ARRAY) && !augment) || + else if ((((v->pm->flags & PM_ARRAY) && !(flags & ASSPM_AUGMENT)) || (v->pm->flags & PM_HASHED)) && !(v->pm->flags & (PM_SPECIAL|PM_TIED)) && unset(KSHARRAYS)) { @@ -2024,13 +2026,18 @@ createparam(t, PM_SCALAR); v = NULL; } + else + flags &= ~ASSPM_WARN_CREATE; } if (!v && !(v = getvalue(&vbuf, &t, 1))) { unqueue_signals(); zsfree(val); return NULL; } - if (augment) { + if ((flags & ASSPM_WARN_CREATE) && v->pm->level == 0) + zwarn("scalar parameter %s created globally in function", + v->pm->nam, 0); + if (flags & ASSPM_AUGMENT) { if (v->start == 0 && v->end == -1) { switch (PM_TYPE(v->pm->flags)) { case PM_SCALAR: @@ -2109,7 +2116,7 @@ /**/ mod_export Param -assignaparam(char *s, char **val, int augment) +assignaparam(char *s, char **val, int flags) { struct value vbuf; Value v; @@ -2127,6 +2134,8 @@ *ss = '\0'; if (!(v = getvalue(&vbuf, &s, 1))) createparam(t, PM_ARRAY); + else + flags &= ~ASSPM_WARN_CREATE; *ss = '['; if (v && PM_TYPE(v->pm->flags) == PM_HASHED) { unqueue_signals(); @@ -2143,7 +2152,7 @@ else if (!(PM_TYPE(v->pm->flags) & (PM_ARRAY|PM_HASHED)) && !(v->pm->flags & (PM_SPECIAL|PM_TIED))) { int uniq = v->pm->flags & PM_UNIQUE; - if (augment) { + if (flags & ASSPM_AUGMENT) { /* insert old value at the beginning of the val array */ char **new; int lv = arrlen(val); @@ -2153,12 +2162,13 @@ memcpy(new+1, val, sizeof(char *) * (lv + 1)); free(val); val = new; - } unsetparam(t); createparam(t, PM_ARRAY | uniq); v = NULL; } + else + flags &= ~ASSPM_WARN_CREATE; } if (!v) if (!(v = fetchvalue(&vbuf, &t, 1, SCANPM_ASSIGNING))) { @@ -2167,7 +2177,10 @@ return NULL; } - if (augment) { + if ((flags & ASSPM_WARN_CREATE) && v->pm->level == 0) + zwarn("array parameter %s created globally in function", + v->pm->nam, 0); + if (flags & ASSPM_AUGMENT) { if (v->start == 0 && v->end == -1) { if (PM_TYPE(v->pm->flags) & PM_ARRAY) { v->start = arrlen(v->pm->gsu.a->getfn(v->pm)); Index: Src/zsh.h =================================================================== RCS file: /cvsroot/zsh/zsh/Src/zsh.h,v retrieving revision 1.75 diff -u -r1.75 zsh.h --- Src/zsh.h 15 Jul 2005 17:41:33 -0000 1.75 +++ Src/zsh.h 8 Aug 2005 13:53:51 -0000 @@ -1361,6 +1361,14 @@ #define setsparam(S,V) assignsparam(S,V,0) #define setaparam(S,V) assignaparam(S,V,0) +/* + * Flags for assignsparam and assignaparam. + */ +enum { + ASSPM_AUGMENT = 1 << 0, + ASSPM_WARN_CREATE = 1 << 1 +}; + /* node for named directory hash table (nameddirtab) */ struct nameddir { @@ -1624,6 +1632,7 @@ UNSET, VERBOSE, VIMODE, + WARNCREATEGLOBAL, XTRACE, USEZLE, DVORAK, -- Peter Stephenson Software Engineer CSR PLC, Churchill House, Cambridge Business Park, Cowley Road Cambridge, CB4 0WZ, UK Tel: +44 (0)1223 692070 ********************************************************************** This email and any files transmitted with it are confidential and intended solely for the use of the individual or entity to whom they are addressed. If you have received this email in error please notify the system manager. **********************************************************************