From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29900 invoked by alias); 4 Dec 2015 10:40:14 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 37302 Received: (qmail 27380 invoked from network); 4 Dec 2015 10:40:11 -0000 X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00 autolearn=ham autolearn_force=no version=3.4.0 X-AuditID: cbfec7f4-f79026d00000418a-d4-56616b2c087f Date: Fri, 04 Dec 2015 10:30:01 +0000 From: Peter Stephenson To: zsh-workers@zsh.org Subject: Re: buggy WARN_CREATE_GLOBAL warning Message-id: <20151204103001.164bb61e@pwslap01u.europe.root.pri> In-reply-to: <20151204025155.GA30607@zira.vinc17.org> References: <20151204025155.GA30607@zira.vinc17.org> Organization: Samsung Cambridge Solution Centre X-Mailer: Claws Mail 3.7.9 (GTK+ 2.22.0; i386-redhat-linux-gnu) MIME-version: 1.0 Content-type: text/plain; charset=US-ASCII Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrILMWRmVeSWpSXmKPExsVy+t/xy7o62YlhBhcX6locbH7I5MDoserg B6YAxigum5TUnMyy1CJ9uwSujAdX97EUNMtWnDv/h62B8atYFyMnh4SAicS18y+YIWwxiQv3 1rN1MXJxCAksZZToOd7CBOHMYJJYO/8cC4RzmlHi99MT7BDOGUaJxom3wfpZBFQlJq+/xgZi swkYSkzdNJsRxBYREJc4u/Y8C4gtLKArcer6ESYQm1fAXuL1zQ9gcU4BU4mnH/+C2UJAN31u Xg5m8wvoS1z9+4kJ4j57iZlXzjBC9ApK/Jh8D6yGWUBLYvO2JlYIW15i85q3zBBz1CVu3N3N PoFReBaSlllIWmYhaVnAyLyKUTS1NLmgOCk911CvODG3uDQvXS85P3cTIySgv+xgXHzM6hCj AAejEg8vw6aEMCHWxLLiytxDjBIczEoivMwyiWFCvCmJlVWpRfnxRaU5qcWHGKU5WJTEeefu eh8iJJCeWJKanZpakFoEk2Xi4JRqYNSImqQu4XSbJdu55pL+FI0uuZlWlqnNDY+kwjOPlxTP f3hGdJfhqpLPL47MVEzbKT4jSSmtfMlluynX8sSyfu6xnbxincWhkxkOJ7gMu9SvRPr/VVl5 4NiKG1fbdtqwLFhx0TTlYpBKf1n7o/rym41L7PivpC5vuVzx6oAKr/Xjy1ycXduM1imxFGck GmoxFxUnAgBZUFDAZAIAAA== On Fri, 4 Dec 2015 03:51:55 +0100 Vincent Lefevre wrote: > setopt WARN_CREATE_GLOBAL > > foo() > { > local blah=$(TZ=UTC date) > } > > foo > ---------------------------------------- > > I get: > > foo:2: scalar parameter TZ created globally in function foo I think the following is logically correct --- variables can't propagate back from forks so you don't care if they're notionally global. However, I don't actually understand why this produces a warning and the normal case foo() { TX=UTC data } foo doesn't. That is, I can see the test that means that the normal case doesn't, but I can't see why the warning becomes active when in the command substitution. Don't read this after a heavy lunch, but I suspect it's some subtlety to do with the fact that we know we're about to exit the subshell after the substitution so aren't set up to restore the variable ("set up to" --- but if we forked we wouldn't actually need to restore, though if it's e.g. a shell function we would), so addvars() doesn't know we're in the export-for-a-command case. If that's right, then this fix ought to be robust because the reason we're not worried about restoring variables is the same reason we don't care about the locallevel. It would almost certainly make more sense to move the locallevel test down to where the ASSPM_WARN_CREATE flag is tested; the trouble is it's quite hard to get your mind round all the possible side effects. pws diff --git a/Src/exec.c b/Src/exec.c index fc31c6b..acc867c 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -2264,7 +2264,7 @@ addvars(Estate state, Wordcode pc, int addflags) * is implicitly scoped. */ flags = (!(addflags & ADDVAR_RESTORE) && - locallevel > 0 && isset(WARNCREATEGLOBAL)) ? + locallevel > forklevel && isset(WARNCREATEGLOBAL)) ? ASSPM_WARN_CREATE : 0; xtr = isset(XTRACE); if (xtr) { diff --git a/Src/params.c b/Src/params.c index 142697f..aed72d4 100644 --- a/Src/params.c +++ b/Src/params.c @@ -2868,7 +2868,7 @@ mod_export Param setsparam(char *s, char *val) { return assignsparam( - s, val, isset(WARNCREATEGLOBAL) && locallevel > 0 ? + s, val, isset(WARNCREATEGLOBAL) && locallevel > forklevel ? ASSPM_WARN_CREATE : 0); } @@ -2966,7 +2966,7 @@ mod_export Param setaparam(char *s, char **aval) { return assignaparam( - s, aval, isset(WARNCREATEGLOBAL) && locallevel >0 ? + s, aval, isset(WARNCREATEGLOBAL) && locallevel > forklevel ? ASSPM_WARN_CREATE : 0); } @@ -2997,7 +2997,7 @@ sethparam(char *s, char **val) if (!(v = fetchvalue(&vbuf, &s, 1, SCANPM_ASSIGNING))) { DPUTS(!v, "BUG: assigning to undeclared associative array"); createparam(t, PM_HASHED); - checkcreate = isset(WARNCREATEGLOBAL) && locallevel > 0; + checkcreate = isset(WARNCREATEGLOBAL) && locallevel > forklevel; } else if (!(PM_TYPE(v->pm->node.flags) & PM_HASHED) && !(v->pm->node.flags & PM_SPECIAL)) { unsetparam(t); @@ -3075,7 +3075,7 @@ setnparam(char *s, mnumber val) /* errflag |= ERRFLAG_ERROR; */ return NULL; } - if (!was_unset && isset(WARNCREATEGLOBAL) && locallevel > 0) + if (!was_unset && isset(WARNCREATEGLOBAL) && locallevel > forklevel) check_warn_create(v->pm, "numeric"); } setnumvalue(v, val); @@ -3113,7 +3113,8 @@ setiparam_no_convert(char *s, zlong val) convbase(buf, val, 10); return assignsparam( s, ztrdup(buf), - isset(WARNCREATEGLOBAL) && locallevel > 0 ? ASSPM_WARN_CREATE : 0); + isset(WARNCREATEGLOBAL) && locallevel > forklevel ? + ASSPM_WARN_CREATE : 0); } /* Unset a parameter */