Parser issues and and [[ $var ]]

Parser issues and and [[ $var ]]

[Bart Schaefer]

On Apr 26, 10:07am, Martin Vaeth wrote:
}
} BTW: "emulate -L bash" would be useful to read some distribution's
} or user's bash setup files. Main difference to emulate -L sh would be
}   setopt ksh_glob no_sh_glob brace_expand
} and that [[ $var ]] is treated equivalent to [[ -n $var ]]:
} I experienced that the latter is the main issue with bash compatibility
} (besides shopt and PS1, of course, which are not reasonable to emulate).

Hmm.  Looks like both bash and ksh93 accept [[ $foo ]] as valid syntax.
This turns out to be surprisingly difficult to fix, and poking at it,
I think we may have broken some things in parse.c.

For example, [ -t ] is supposed to be equivalent to [ -t 1 ] and there
is code late in par_cond_2() that handles this, but that code is never
reached because the /* POSIX 1003.2 */ code at the top of par_cond_2()
preemptively turns it into [ -n -t ].

The same unreached code for [ -t ] also tries to handle [ $var ] but is
not usable for [[ $var ]] because of the different lexing conventions
for "test"/"[" vs. [[ ... ]].  Also [[ -t ]] is NOT supposed to behave
like either of [[ -t 1 ]] or [[ -n -t ]], so that part of the code in
question must be avoided.

Also, if you compare the later code to the earlier code, sometimes we
call dupstring() on the first argument to par_cond_double() and other
times we do not.  Either we don't need dupstring(), or not using it is
a bug waiting to happen.

So it looks like par_cond_2() needs some more overhauling.

More [[ $var ]] (Re: zsh-newuser-install)

[Bart Schaefer]

[> workers]

On Apr 26,  9:21pm, Peter Stephenson wrote:
} Subject: Re: zsh-newuser-install
}
}      condlex();
} +    if (tok == DOUTBRACK)
} +    {
} +	/* allow [[ $testsize ]] for compatibility */
} +	return par_cond_double(dupstring("-n"), s1);
} +    }


This isn't good enough, [[ $var && $othervar ]] should also work, etc.

Re: Parser issues and and [[ $var ]]

[Bart Schaefer]

Looking at this a bit further ...

On Apr 26,  1:30pm, Bart Schaefer wrote:
} Subject: Parser issues and and [[ $var ]]
}
} Hmm.  Looks like both bash and ksh93 accept [[ $foo ]] as valid syntax.
} This turns out to be surprisingly difficult to fix, and poking at it,
} I think we may have broken some things in parse.c.
} 
} For example, [ -t ] is supposed to be equivalent to [ -t 1 ] and there
} is code late in par_cond_2() that handles this, but that code is never
} reached because the /* POSIX 1003.2 */ code at the top of par_cond_2()
} preemptively turns it into [ -n -t ].

Further prodding seems to indicate that bash and ksh disagree on this
point.  [ -t ] in bash (and in the "test" external command) is treated
like [ "-t" ], so

    [ -t ] >/dev/null

is true in bash3.2 but false in ksh93 (including builtin test).  As far
as I can tell, this is the only operator that behaves this way, both
bash and ksh treat all other binary operators as strings when they
have no argument.

Zsh has traditionally had the ksh behavior here, though as noted this
was probably unintentionally changed a couple of revisions ago.

Ksh93 has one other interesting feature that I wasn't able to make work
in zsh:

    ksh$ [ -t ] >/dev/null || print tested FD 1 not a terminal
    tested FD 1 not a terminal

    ksh$ x=t
    ksh$ [ -$x ] >/dev/null && print tested as a string
    tested as a string

So it appears ksh is tokenizing the arguments of "[" (and builtin test),
whereas zsh will expand $x first in that case.  This is similar to ksh
treating "typeset" as special in command position.

} The same unreached code for [ -t ] also tries to handle [ $var ] but is
} not usable for [[ $var ]] because of the different lexing conventions
} for "test"/"[" vs. [[ ... ]].  Also [[ -t ]] is NOT supposed to behave
} like either of [[ -t 1 ]] or [[ -n -t ]], so that part of the code in
} question must be avoided.

That code turns out to be reachable after all if the expression is more
complex than just [ -a ], e.g. [ -a -a -a ].  This plus examination of
ecadd() leads me to belive that ...

} Also, if you compare the later code to the earlier code, sometimes we
} call dupstring() on the first argument to par_cond_double() and other
} times we do not.  Either we don't need dupstring(), or not using it is
} a bug waiting to happen.

... calling dupstring() on literals before passing to par_cond_double()
is the right thing to do.

I think the following produces syntax errors in all the places it should
and works in the rest of the cases, but I have not yet tested it with
new conditionals added by modules -- e.g., the recomputation of dble
for s2 (last hunk) may need knowledge of added conditions.  So I won't
commit until others have a chance to play with this.


diff --git a/Src/parse.c b/Src/parse.c
index 530a070..6ea997d 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -2068,6 +2068,9 @@ par_cond_2(void)
 	    /* one argument: [ foo ] is equivalent to [ -n foo ] */
 	    s1 = tokstr;
 	    condlex();
+	    /* ksh behavior: [ -t ] means [ -t 1 ]; bash disagrees */
+	    if (unset(POSIXBUILTINS) && !strcmp(s1, "-t"))
+		return par_cond_double(s1, dupstring("1"));
 	    return par_cond_double(dupstring("-n"), s1);
 	}
 	if (testargs[1]) {
@@ -2086,6 +2089,9 @@ par_cond_2(void)
 		return par_cond_triple(s1, s2, s3);
 	    }
 	}
+	/*
+	 * We may fall through here in oddball cases like [ = -a ! ]
+	 */
     }
     if (tok == BANG) {
 	/*
@@ -2114,18 +2120,17 @@ par_cond_2(void)
 	condlex();
 	return r;
     }
+    if ((s1 = tokstr) != 0)
+	dble = (*s1 == '-' && strspn(s1+1, "abcdefghknoprstuwxzLONGS") == 1
+		&& !s1[2]);
     if (tok != STRING) {
-	if (tok && tok != LEXERR && condlex == testlex) {
-	    s1 = tokstr;
+	/* Check first argument for [[ string ]] re-interpretation */
+	if (tokstr && tok != LEXERR && (!dble || condlex == testlex)) {
 	    condlex();
-	    return par_cond_double("-n", s1);
+	    return par_cond_double(dupstring("-n"), s1);
 	} else
 	    YYERROR(ecused);
     }
-    s1 = tokstr;
-    if (condlex == testlex)
-	dble = (*s1 == '-' && strspn(s1+1, "abcdefghknoprstuwxzLONGS") == 1
-		  && !s1[2]);
     condlex();
     if (tok == INANG || tok == OUTANG) {
 	enum lextok xtok = tok;
@@ -2140,15 +2145,19 @@ par_cond_2(void)
 	return 1;
     }
     if (tok != STRING) {
-	if (tok != LEXERR && condlex == testlex) {
-	    if (!dble)
-		return par_cond_double("-n", s1);
-	    else if (!strcmp(s1, "-t"))
-		return par_cond_double(s1, "1");
+	/*
+	 * Check second argument in case semantics e.g. [ = -a = ]
+	 * mean we have to go back and fix up the first one
+	 */
+	if (tok != LEXERR) {
+	    if (!dble || condlex == testlex)
+		return par_cond_double(dupstring("-n"), s1);
 	} else
 	    YYERROR(ecused);
     }
-    s2 = tokstr;
+    if ((s2 = tokstr) != 0 && condlex != testlex)
+	dble = (*s2 == '-' && strspn(s2+1, "abcdefghknoprstuwxzLONGS") == 1
+		&& !s2[2]);
     incond++;			/* parentheses do globbing */
     condlex();
     incond--;			/* parentheses do grouping */

Re: Parser issues and and [[ $var ]]

[Bart Schaefer]

I've found a couple of other bugs with 32604, so it's probably just as
well that I didn't commit/push it.  Does anyone know why the lexer
sometimes sets (tok = DOUTBRACK, tokstr = "\220\220") and other times
(tok = DOUTBRACK, tokstr = NULL) ?

It would seem as though it ought to be consistent.  It does seem to
consistently "return" (tok = DAMPER, tokstr = NULL) for example.

On May 10,  2:09pm, Bart Schaefer wrote:
}
}     [ -t ] >/dev/null
} 
} is true in bash3.2 but false in ksh93 (including builtin test).  As far
} as I can tell, this is the only operator that behaves this way, both
} bash and ksh treat all other binary operators as strings when they
} have no argument.

Of course I meant to write "prefix operators" not "binary".

} I think the following produces syntax errors in all the places it should
} and works in the rest of the cases, but I have not yet tested it with
} new conditionals added by modules -- e.g., the recomputation of dble
} for s2 (last hunk) may need knowledge of added conditions.

With the patch from 32604 in place, I added a simple prefix operator to
the example module:

    CONDDEF("m", 0, cond_p_m, 0, -1, 0),

This should accept any number of arguments including none.  I created
cond_p_m() to return 0 if there are no arguments and 1 if there are
any arguments (recall that the C function return values are inverted
from the shell true/false return status).

After recompiling and loading zsh/example, [[ -m ]] returns true.  GDB
confirms that cond_p_m() is never being called.  The condition callback
is correctly invoked for [[ -m 1 2 ]] etc.

On the other hand, WITHOUT 32604, [[ -m ]] is a parse error, so it would
appear that prefix operators are not allowed to have no arguments.  This
should probably be documented.

Back on the first hand, in ksh93 [[ -z ]] is a parse error (that is, a
prefix operator is never tested as a plain string except in "test")
whereas [[ -m ]] is not (because there is no -m operator).  So it would
seem that once an operator is defined, it should not be allowed to
parse as a string.

Thus it would seem that the parser does need a way to explicitly test
for module-defined operators to also support non-operator non-empty
strings evaluating as true.  Or, we can decree that any string that
starts with a "-" is treated as an operator, since all module-defined
operators must start with "-", which would differ from ksh93 but not
from previous zsh.  (foo=-m; [[ $foo ]]) would still test as a string.

Re: Parser issues and and [[ $var ]]

[Peter Stephenson]

On Sat, 10 May 2014 18:01:44 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> I've found a couple of other bugs with 32604, so it's probably just as
> well that I didn't commit/push it.  Does anyone know why the lexer
> sometimes sets (tok = DOUTBRACK, tokstr = "\220\220") and other times
> (tok = DOUTBRACK, tokstr = NULL) ?

It must surely be an oversight.  I think a long time ago the lexer
didn't bother setting tokstr when there was a unique token, since the string could in principle be deduced; however, it proverd much more convenient just to be
able to use tokstr in any case.  So it probably hasn't been updated properly.
 
> Thus it would seem that the parser does need a way to explicitly test
> for module-defined operators to also support non-operator non-empty
> strings evaluating as true.  Or, we can decree that any string that
> starts with a "-" is treated as an operator, since all module-defined
> operators must start with "-", which would differ from ksh93 but not
> from previous zsh.  (foo=-m; [[ $foo ]]) would still test as a string.

I think the latter is probably acceptable, but maybe there should be an
interface saying a condition code defined by a module has been called with
zero arguments (which might on failure alternatively confirm that there
was no such condition code, allowing a different interpretation).

pws

Re: Parser issues and and [[ $var ]]

[Bart Schaefer]

On May 11,  6:01pm, Peter Stephenson wrote:
} Subject: Re: Parser issues and and [[ $var ]]
}
} On Sat, 10 May 2014 18:01:44 -0700
} Bart Schaefer <schaefer@brasslantern.com> wrote:
} > I've found a couple of other bugs with 32604, so it's probably just as
} > well that I didn't commit/push it.  Does anyone know why the lexer
} > sometimes sets (tok = DOUTBRACK, tokstr = "\220\220") and other times
} > (tok = DOUTBRACK, tokstr = NULL) ?
} 
} It must surely be an oversight. I think a long time ago the lexer
} didn't bother setting tokstr when there was a unique token, since the
} string could in principle be deduced; however, it proverd much more
} convenient just to be able to use tokstr in any case.

In this instance it's actually easier for the caller if tokstr = NULL
consistently, rather than the other way around ...

} > Thus it would seem that the parser does need a way to explicitly test
} > for module-defined operators to also support non-operator non-empty
} > strings evaluating as true.  Or, we can decree that any string that
} > starts with a "-" is treated as an operator, since all module-defined
} > operators must start with "-", which would differ from ksh93 but not
} > from previous zsh.  (foo=-m; [[ $foo ]]) would still test as a string.
} 
} I think the latter is probably acceptable, but maybe there should be an
} interface saying a condition code defined by a module has been called with
} zero arguments (which might on failure alternatively confirm that there
} was no such condition code, allowing a different interpretation).

Hmm.

It turns out that because you can define a function that uses an operator
before you have loaded the module ...

zsh% foo() { [[ -m 1 2 3 ]] && echo OK }
zsh% foo
zsh: unknown condition: -m
zsh% zmodload zsh/example
The example module has now been set up.
zsh% foo
OK

... that it also won't work to cause a parse error on unknown operators.

I'm beginning to suspect that from a syntax perspective prefix operators
that accept multiple arguments are already a problem.  Suppose you have
defined an operator that takes 0 to 4 arguments, call it "-uptofour".
Isn't the following an intractable syntax problem?

  [[ -uptofour -a -n $foo ]]

Or do the "arguments" have to be things that don't look like operators?
What if another module defines an infix operator "-both" and you write

  [[ -uptofour -both -n $foo ]]

?  Is -both an operator or an argument?  Before the module is loaded, how
do you tell?

Incidentally, even before any of my patching attempts:

zsh% [[ -n 1 2 3 ]]
zsh: unknown condition: -n

Not parse error, not "wrong number of arguments", but "uknown condition".
Whereas:

$ [[ -n 1 2 3 ]]
ksh: syntax error: `2' unexpected

[PATCH] Re: Parser issues and and [[ $var ]]

[Bart Schaefer]

On May 11, 11:12am, Bart Schaefer wrote:
}
} } > Does anyone know why the lexer
} } > sometimes sets (tok = DOUTBRACK, tokstr = "\220\220") and other times
} } > (tok = DOUTBRACK, tokstr = NULL) ?
} } 
} } It must surely be an oversight.

I've left the lexer unchanged and accounted for it in par_cond_2().
 
} } > Or, we can decree that any string that
} } > starts with a "-" is treated as an operator
} } 
} } I think the latter is probably acceptable

The patch below does this, which means a lot of runtime "unknown" errors
rather than parse errors.

} } but maybe there should be an
} } interface saying a condition code defined by a module has been called with
} } zero arguments (which might on failure alternatively confirm that there
} } was no such condition code, allowing a different interpretation).

I haven't done anything specific about this yet except to allow conditions
to be called with zero arguments.  We could change the "unknown condition"
handling in cond.c if we want this to behave differently.
 
} Isn't the following an intractable syntax problem?
} 
}   [[ -uptofour -a -n $foo ]]
} 
} Or do the "arguments" have to be things that don't look like operators?
} What if another module defines an infix operator "-both" and you write
} 
}   [[ -uptofour -both -n $foo ]]
} 
} ?  Is -both an operator or an argument?  Before the module is loaded, how
} do you tell?

This problem already exists -- infix operators that aren't tokens like
&& and || are already lower prececence than prefix operators.  E.g.:

zsh% [[ -n 1 -eq 1 ]]
zsh: unknown condition: -n

So I don't think the patch below changes any of that.

I've included PWS's test from users/18775.  Some more tests of *wrong*
syntax might be helpful; for example, Test/A01grammar.ztst helped find
a crash on tokstr == NULL with the "echo '[[' >bad_syntax" test, though
the current testing regime makes it pretty difficult to determine what
test failed when the failure was a segmentation fault. 

diff --git a/Src/parse.c b/Src/parse.c
index 530a070..1af98b5 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -2068,6 +2068,9 @@ par_cond_2(void)
 	    /* one argument: [ foo ] is equivalent to [ -n foo ] */
 	    s1 = tokstr;
 	    condlex();
+	    /* ksh behavior: [ -t ] means [ -t 1 ]; bash disagrees */
+	    if (unset(POSIXBUILTINS) && !strcmp(s1, "-t"))
+		return par_cond_double(s1, dupstring("1"));
 	    return par_cond_double(dupstring("-n"), s1);
 	}
 	if (testargs[1]) {
@@ -2086,6 +2089,10 @@ par_cond_2(void)
 		return par_cond_triple(s1, s2, s3);
 	    }
 	}
+	/*
+	 * We fall through here on any non-numeric infix operator
+	 * or any other time there are at least two arguments.
+	 */
     }
     if (tok == BANG) {
 	/*
@@ -2114,18 +2121,20 @@ par_cond_2(void)
 	condlex();
 	return r;
     }
+    s1 = tokstr;
+    dble = (s1 && *s1 == '-'
+	    && (condlex != testlex
+		|| strspn(s1+1, "abcdefghknoprstuwxzLONGS") == 1)
+	    && !s1[2]);
     if (tok != STRING) {
-	if (tok && tok != LEXERR && condlex == testlex) {
-	    s1 = tokstr;
+	/* Check first argument for [[ STRING ]] re-interpretation */
+	if (s1 /* tok != DOUTBRACK && tok != DAMPER && tok != DBAR */
+	    && tok != LEXERR && (!dble || condlex == testlex)) {
 	    condlex();
-	    return par_cond_double("-n", s1);
+	    return par_cond_double(dupstring("-n"), s1);
 	} else
 	    YYERROR(ecused);
     }
-    s1 = tokstr;
-    if (condlex == testlex)
-	dble = (*s1 == '-' && strspn(s1+1, "abcdefghknoprstuwxzLONGS") == 1
-		  && !s1[2]);
     condlex();
     if (tok == INANG || tok == OUTANG) {
 	enum lextok xtok = tok;
@@ -2140,15 +2149,21 @@ par_cond_2(void)
 	return 1;
     }
     if (tok != STRING) {
-	if (tok != LEXERR && condlex == testlex) {
-	    if (!dble)
-		return par_cond_double("-n", s1);
-	    else if (!strcmp(s1, "-t"))
-		return par_cond_double(s1, "1");
+	/*
+	 * Check second argument in case semantics e.g. [ = -a = ]
+	 * mean we have to go back and fix up the first one
+	 */
+	if (tok != LEXERR) {
+	    if (!dble || condlex == testlex)
+		return par_cond_double(dupstring("-n"), s1);
+	    else
+		return par_cond_multi(s1, newlinklist());
 	} else
 	    YYERROR(ecused);
     }
-    s2 = tokstr;
+    s2 = tokstr;   
+    if (condlex != testlex)
+	dble = (s2 && s2 && *s2 == '-' && !s2[2]);
     incond++;			/* parentheses do globbing */
     condlex();
     incond--;			/* parentheses do grouping */
diff --git a/Test/C02cond.ztst b/Test/C02cond.ztst
index 94fca8b..6900147 100644
--- a/Test/C02cond.ztst
+++ b/Test/C02cond.ztst
@@ -349,6 +349,14 @@ F:Failures in these cases do not indicate a problem in the shell.
 >0
 >1
 
+  foo=''
+  [[ $foo ]] || print foo is empty
+  foo=full
+  [[ $foo ]] && print foo is full
+0:bash compatibility with single [[ ... ]] argument
+>foo is empty
+>foo is full
+
 %clean
   # This works around a bug in rm -f in some versions of Cygwin
   chmod 644 unmodish

Re: [PATCH] Re: Parser issues and and [[ $var ]]

[Peter Stephenson]

On Mon, 12 May 2014 22:01:12 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> This problem already exists -- infix operators that aren't tokens like
> && and || are already lower prececence than prefix operators.  E.g.:
> 
> zsh% [[ -n 1 -eq 1 ]]
> zsh: unknown condition: -n
> 
> So I don't think the patch below changes any of that.

While this sort of thing and the problems with modular conditions are a
real headache for parsing, I don't think the actual practical problems
are anything like as bad.  Almost always real tests use parameters or
quoted expressions (you don't need to know if the literal string -n has
non-zero length), and since [[, unlike [, is parsed before expansion
such tests work fine.  About the only likely exception I can think of is
where someone's used an eval to get an extra level of expansion --- but
that's always a bit hairy.

So having addressed the compatibility issues, perhaps we should restrict
ourselves to tweaking the documentation to mention the problems and how
to keep them at bay.

pws

Re: [PATCH] Re: Parser issues and and [[ $var ]]

[Bart Schaefer]

On May 13, 10:10am, Peter Stephenson wrote:
} Subject: Re: [PATCH] Re: Parser issues and and [[ $var ]]
}
} > This problem already exists -- infix operators that aren't tokens like
} > && and || are already lower prececence than prefix operators.
} 
} While this sort of thing and the problems with modular conditions are a
} real headache for parsing, I don't think the actual practical problems
} are anything like as bad.

It's almost certainly not a problem for correct programs.  The case I'd
be worried about is a program that ksh would reject as syntactically bad
(and therefore never start running) where zsh would accept it and then
fail at run time (possibly creating side-effects).

However, having had this situation already for a rather long time, I am
not particularly worked up about it.