Re: export "a=b"=~

[Peter Stephenson <p.stephenson@xxxxxxxxxxx>]

On Wed, 8 Mar 2017 13:23:45 +0000
Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx> wrote:
> Stephane Chazelas wrote on Wed, Mar 08, 2017 at 08:50:00 +0000:
> > Not that it's likely to ever be a concern, but I just noticed:
> > 
> > $ zsh -c 'export "a"=a=~; echo $a'
> > a=~
> > $ zsh -c 'export "a=a"=~; echo $a'
> > a=/home/chazelas
> > $ zsh -c 'export a=a=~; echo $a'
> > a=~
> > 
> > (5.1.1)
> 
> [ I assume the point is that tilde expansion wasn't expected in the
> second case. ]
> 
> Same behaviour in latest master.  This is because the MAGIC_EQUAL_SUBST
> option, even when unset, implicitly applies to "export".  (The
> declaration of "export" in builtins.c has the BINF_MAGICEQUALS flag
> since before CVS.)
> 
> Should the behaviour be changed in 'emulate sh' / 'setopt posix*' mode?

I think that flag is now largely redundant since these days the "~"
expansion is normally triggered by parsing the variable and argument
separately, in which case the older "magic" behaviour isn't useful.

Hence, regardless of compatibility mode, we should probably suppress
that flag in this case, leaving the fix-up magic behaviour for when we
encounter a builtin without recognising it as a keyword in the parser
(an unusual case where it's OK to make reasonable guesses and
compatibility with older zsh is probably the right answer).

So I think the following ought to work without any significant side
effects.  I've renamed the controlling variable for clarity; the older,
more general, name is now purely historical.

One extra thing: it seems likely that you don't want MAGIC_EQUAL_SUBST
behaviour *at all* in the case where you recognised the keyword,
i.e. even if the option is explicitly set, right?  So I've changed that,
too.  In other words,

setopt magicequalsubst
typeset "a=a"=~

*still* only assigns the value 'a=~'.  See comment I've added for the full
logic.

It would certainly be possible not to set magic_assign if the
POSIX_BUILTINS option is on, too, but that's now a very minor effect.

pws

diff --git a/Src/exec.c b/Src/exec.c
index 8b32246..ec415cf 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -2658,7 +2658,7 @@ execcmd_exec(Estate state, Execcmd_params eparams,
     char *text;
     int save[10];
     int fil, dfil, is_cursh, do_exec = 0, redir_err = 0, i;
-    int nullexec = 0, assign = 0, forked = 0;
+    int nullexec = 0, magic_assign = 0, forked = 0;
     int is_shfunc = 0, is_builtin = 0, is_exec = 0, use_defpath = 0;
     /* Various flags to the command. */
     int cflags = 0, orig_cflags = 0, checked = 0, oautocont = -1;
@@ -2761,7 +2761,8 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 		/* autoload the builtin if necessary */
 		if (!(hn = resolvebuiltin(cmdarg, hn)))
 		    return;
-		assign = (hn->flags & BINF_MAGICEQUALS);
+		if (type != WC_TYPESET)
+		    magic_assign = (hn->flags & BINF_MAGICEQUALS);
 		break;
 	    }
 	    checked = 0;
@@ -2929,8 +2930,22 @@ execcmd_exec(Estate state, Execcmd_params eparams,
     if (noerrexit == 2 && !is_shfunc)
 	noerrexit = 0;
 
-    /* Do prefork substitutions */
-    esprefork = (assign || isset(MAGICEQUALSUBST)) ? PREFORK_TYPESET : 0;
+    /* Do prefork substitutions.
+     *
+     * Decide if we need "magic" handling of ~'s etc. in
+     * assignment-like arguments.
+     * - If magic_assign is set, we are using a builtin of the
+     *   tyepset family, but did not recognise this as a keyword,
+     *   so need guess-o-matic behaviour.
+     * - Otherwise, if we did recognise the keyword, we never need
+     *   guess-o-matic behaviour as the argument was properly parsed
+     *   as such.
+     * - Otherwise, use the behaviour specified by the MAGIC_EQUAL_SUBST
+     *   option.
+     */
+    esprefork = (magic_assign ||
+		 (isset(MAGICEQUALSUBST) && type != WC_TYPESET)) ?
+		 PREFORK_TYPESET : 0;
     if (args && eparams->htok)
 	prefork(args, esprefork, NULL);
 
@@ -3710,7 +3725,7 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 		     * Save if it's got "command" in front or it's
 		     * not a magic-equals assignment.
 		     */
-		    if ((cflags & (BINF_COMMAND|BINF_ASSIGN)) || !assign)
+		    if ((cflags & (BINF_COMMAND|BINF_ASSIGN)) || !magic_assign)
 			do_save = 1;
 		}
 		if (do_save && varspc)