Surprising parsing result with anonymous functions and for loops

Surprising parsing result with anonymous functions and for loops

[Mikael Magnusson]

The intended command was something along these lines:
() { for a { echo $a } } some words here
but I forgot the enclosing { } and wrote the following
() for a { echo $a } some words here
surely this doesn't work, right?... wrong:
% () for a { echo $a } some words here
some
words
here

Perhaps even more surprising is the following:
% () for a { echo $a } ls
ls
--color=auto
-T
0
-A
-v
--quoting-style=shell

I haven't looked at the parsing for the anonymous function stuff, but
if it's not too hairy to fix, my vote is we drop this easter egg at
some point.

-- 
Mikael Magnusson

Re: Surprising parsing result with anonymous functions and for loops

[Peter Stephenson]

On Wed, 24 Sep 2014 16:07:36 +0200
Mikael Magnusson <mikachu@gmail.com> wrote:
> I haven't looked at the parsing for the anonymous function stuff, but
> if it's not too hairy to fix, my vote is we drop this easter egg at
> some point.

Unfortunately I think defining functions without braces is part of the
standard, though little used these days.  This would have to be a
special case for anonymous functions, which is less useful.

pws

Fwd: Surprising parsing result with anonymous functions and for loops

[Clint Hepner]

[-- Attachment #1: Type: text/plain, Size: 2064 bytes --]

I meant to reply to the list, but only replied to Mikael instead.

---------- Forwarded message ----------
From: Clint Hepner <clint.hepner@gmail.com>
Date: Wed, Sep 24, 2014 at 3:11 PM
Subject: Re: Surprising parsing result with anonymous functions and for
loops
To: Mikael Magnusson <mikachu@gmail.com>


The POSIX standard defines a function definition as

    <name>  () <compound-statement>

where { ...; } is just one kind of compound statement, along with for
loops, while loops, etc. The following are all valid function definitions:

    foo () { echo bar; }
    foo () while [[ $i != foo ]]; do i=foo; done
    foo () for i in 1 2 3; do echo $i; done
    foo () ( echo bar; )
    foo () (( x=3 ))

The man page is ambiguous about what constitutes a function; it lists three
allowable forms:

       function word ... [ () ] [ term ] { list }
       word ... () [ term ] { list }
       word ... () [ term ] command

where the third is the one that describes the observed behavior.  I thought
the first might imply
special behavior when using the function keyword, but

    function foo (( x = 3))

works as well. The second is would seem to be a special case of the third,
except it does seem to treat the braces as part of the syntax:

    foo () { echo foo }

would in POSIX require a semicolon prior to the closing brace, but works in
zsh.

On Wed, Sep 24, 2014 at 10:07 AM, Mikael Magnusson <mikachu@gmail.com>
wrote:

> The intended command was something along these lines:
> () { for a { echo $a } } some words here
> but I forgot the enclosing { } and wrote the following
> () for a { echo $a } some words here
> surely this doesn't work, right?... wrong:
> % () for a { echo $a } some words here
> some
> words
> here
>
> Perhaps even more surprising is the following:
> % () for a { echo $a } ls
> ls
> --color=auto
> -T
> 0
> -A
> -v
> --quoting-style=shell
>
> I haven't looked at the parsing for the anonymous function stuff, but
> if it's not too hairy to fix, my vote is we drop this easter egg at
> some point.
>
> --
> Mikael Magnusson
>

Re: Surprising parsing result with anonymous functions and for loops

[Mikael Magnusson]

On 24 September 2014 16:26, Peter Stephenson <p.stephenson@samsung.com> wrote:
> On Wed, 24 Sep 2014 16:07:36 +0200
> Mikael Magnusson <mikachu@gmail.com> wrote:
>> I haven't looked at the parsing for the anonymous function stuff, but
>> if it's not too hairy to fix, my vote is we drop this easter egg at
>> some point.
>
> Unfortunately I think defining functions without braces is part of the
> standard, though little used these days.  This would have to be a
> special case for anonymous functions, which is less useful.
>
> pws

Yes, I'm aware of that, and it's not a problem at all for normal functions;
% foo() for a { echo $a } ls
zsh: parse error near `ls'

Just in case something is unclear, let me try and clarify.
% () echo inside the function; echo outside the function
inside the function
outside the function

% foo () echo defining the function, $@; foo calling the function
defining the function, calling the function

The above two are perfectly okay. Now let's see when there's no
function involved,
% for a (foo bar) { echo $a }
foo
bar
% for a (foo bar) { echo $a } bing baz
zsh: correct 'bing' to 'big' [nyae]? n
zsh: parse error near `bing'

Okay, so an anonymous function without braces should not possibly be
able to take arguments, because they are either part of the single
command in the function, or if you have a ;, they are not part of the
function definition at all. And writing anything after the end of a
for loop without a separator is also clearly a syntax error. Now comes
the surprising part again and hopefully this time it is surprising,
% () for a { echo $a } bing baz
zsh: correct 'bing' to 'big' [nyae]? n
bing
baz

And again, just to clarify, this is still correctly rejected:
% foo () for a { echo $a } bing baz
zsh: correct 'bing' to 'big' [nyae]? n
zsh: parse error near `bing'


-- 
Mikael Magnusson

Re: Surprising parsing result with anonymous functions and for loops

[Bart Schaefer]

On Wed, Sep 24, 2014 at 3:02 PM, Mikael Magnusson <mikachu@gmail.com> wrote:
>
> Okay, so an anonymous function without braces should not possibly be
> able to take arguments, because they are either part of the single
> command in the function, or if you have a ;, they are not part of the
> function definition at all.

That's not quite how it works.  The foo() case is a function
definition, but the "anonymous" case is simultaneously a definition
and a call.  In the latter situation, the parser consumes a valid
function definition, and then anything left over is its arguments.
Since "for a { echo $a }" is a valid function body, the parse switches
to looking for arguments after that point.  In the former case, there
is no implicit function call, so it is an error for anything to be
left over.

The presence of braces around the entire body simply allows the parser
to distinguish the body from the arguments.  If there are other
syntactic clues, those also distinguish body from arguments, e.g., the
token "done" here:

() for a; do echo $a; done bing bong

Note that the semicolons didn't terminate the function definition,
because they are part of the for-do-done compound syntax.

Re: Surprising parsing result with anonymous functions and for loops

[Bart Schaefer]

> The presence of braces around the entire body simply allows the parser
> to distinguish the body from the arguments.  If there are other
> syntactic clues, those also distinguish body from arguments

Incidentally, I think this is related to zsh's non-standard handling
of redirections following a function definition.  POSIX says that

foo() { echo foo; } >/tmp/foo

should behave the same as

foo() { { echo foo; } >/tmp/foo }

but in zsh it behaves like

{ foo() { echo foo; } } >/tmp/foo

Re: Surprising parsing result with anonymous functions and for loops

[Mikael Magnusson]

On 25 September 2014 07:35, Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Wed, Sep 24, 2014 at 3:02 PM, Mikael Magnusson <mikachu@gmail.com> wrote:
>>
>> Okay, so an anonymous function without braces should not possibly be
>> able to take arguments, because they are either part of the single
>> command in the function, or if you have a ;, they are not part of the
>> function definition at all.
>
> That's not quite how it works.  The foo() case is a function
> definition, but the "anonymous" case is simultaneously a definition
> and a call.  In the latter situation, the parser consumes a valid
> function definition, and then anything left over is its arguments.
> Since "for a { echo $a }" is a valid function body, the parse switches
> to looking for arguments after that point.  In the former case, there
> is no implicit function call, so it is an error for anything to be
> left over.
>
> The presence of braces around the entire body simply allows the parser
> to distinguish the body from the arguments.  If there are other
> syntactic clues, those also distinguish body from arguments, e.g., the
> token "done" here:
>
> () for a; do echo $a; done bing bong
>
> Note that the semicolons didn't terminate the function definition,
> because they are part of the for-do-done compound syntax.

Okay, but does anyone at least agree that doing alias expansion at
that point is highly surprising?

-- 
Mikael Magnusson

Re: Surprising parsing result with anonymous functions and for loops

[Peter Stephenson]

On Thu, 25 Sep 2014 12:02:55 +0200
Mikael Magnusson <mikachu@gmail.com> wrote:
> Okay, but does anyone at least agree that doing alias expansion at
> that point is highly surprising?

You mean regardless of function behaviour?

% alias foo=bar
% for i in 1; do : ; done foo
zsh: parse error near `bar'

The parser doesn't expect anything there so doesn't make special
arrangements not to treat it as a command word because it doesn't mean
anything anyway, but this one is easy to fix.  The cases with standard
keywords are the simple ones as there must be a separator next ---
closing braces are harder because there are so many cases, but I think
loops are unproblematic (this is special to zsh).

Anything similar requires some different ad hoc change.  I had a go at
if and there are gotchas:

  if (true) { : } else { : }

is valid syntax (again this is annoying zsh-specific stuff I wish we
didn't have to maintain) so we need to be in command position after any
closing braces except after the final "else".  We've already got a
reasonable set of tests in this area for the more important non-error
case.

"while" and "repeat" look similar to "for".

The more standard uses of braces for functions are explicitly setting
incmdpos = 1 --- I can't remember why, but I've a vague memory it's
complicated.  It looks like I added a special case for anonymous
functions to set incmdpos = 0.  So you still get

% alias foo=bar
% foo() { : } foo
zsh: parse error near `bar'

and I'm not sure how much I want to look --- it would be easy to break
something here.  If we fix redirections at this point the answer may
become more obvious.

For current shell { ... } this also happens but it's a bit more
understandable as an "always" keyword can follow.

pws


diff --git a/Src/parse.c b/Src/parse.c
index 5f1303f..3633417 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -997,17 +997,20 @@ par_for(int *complex)
 	par_save_list(complex);
 	if (tok != DONE)
 	    YYERRORV(oecused);
+	incmdpos = 0;
 	zshlex();
     } else if (tok == INBRACE) {
 	zshlex();
 	par_save_list(complex);
 	if (tok != OUTBRACE)
 	    YYERRORV(oecused);
+	incmdpos = 0;
 	zshlex();
     } else if (csh || isset(CSHJUNKIELOOPS)) {
 	par_save_list(complex);
 	if (tok != ZEND)
 	    YYERRORV(oecused);
+	incmdpos = 0;
 	zshlex();
     } else if (unset(SHORTLOOPS)) {
 	YYERRORV(oecused);
@@ -1186,9 +1189,12 @@ par_if(int *complex)
     for (;;) {
 	xtok = tok;
 	cmdpush(xtok == IF ? CS_IF : CS_ELIF);
-	zshlex();
-	if (xtok == FI)
+	if (xtok == FI) {
+	    incmdpos = 0;
+	    zshlex();
 	    break;
+	}
+	zshlex();
 	if (xtok == ELSE)
 	    break;
 	while (tok == SEPER)
@@ -1229,6 +1235,7 @@ par_if(int *complex)
 		YYERRORV(oecused);
 	    }
 	    ecbuf[pp] = WCB_IF(type, ecused - 1 - pp);
+	    /* command word (else) allowed to follow immediately */
 	    zshlex();
 	    incmdpos = 1;
 	    if (tok == SEPER)
@@ -1266,6 +1273,7 @@ par_if(int *complex)
 		YYERRORV(oecused);
 	    }
 	}
+	incmdpos = 0;
 	ecbuf[pp] = WCB_IF(WC_IF_ELSE, ecused - 1 - pp);
 	zshlex();
 	cmdpop();
@@ -1296,12 +1304,14 @@ par_while(int *complex)
 	par_save_list(complex);
 	if (tok != DONE)
 	    YYERRORV(oecused);
+	incmdpos = 0;
 	zshlex();
     } else if (tok == INBRACE) {
 	zshlex();
 	par_save_list(complex);
 	if (tok != OUTBRACE)
 	    YYERRORV(oecused);
+	incmdpos = 0;
 	zshlex();
     } else if (isset(CSHJUNKIELOOPS)) {
 	par_save_list(complex);
@@ -1340,12 +1350,14 @@ par_repeat(int *complex)
 	par_save_list(complex);
 	if (tok != DONE)
 	    YYERRORV(oecused);
+	incmdpos = 0;
 	zshlex();
     } else if (tok == INBRACE) {
 	zshlex();
 	par_save_list(complex);
 	if (tok != OUTBRACE)
 	    YYERRORV(oecused);
+	incmdpos = 0;
 	zshlex();
     } else if (isset(CSHJUNKIELOOPS)) {
 	par_save_list(complex);

Re: Surprising parsing result with anonymous functions and for loops

[Mikael Magnusson]

On 25 September 2014 13:39, Peter Stephenson <p.stephenson@samsung.com> wrote:
> On Thu, 25 Sep 2014 12:02:55 +0200
> Mikael Magnusson <mikachu@gmail.com> wrote:
>> Okay, but does anyone at least agree that doing alias expansion at
>> that point is highly surprising?
>
> You mean regardless of function behaviour?
>
> % alias foo=bar
> % for i in 1; do : ; done foo
> zsh: parse error near `bar'

Ah, when i tested this i didn't realize it always happened, since i
tested with my ls alias (so the error said `ls' regardless). So yes
and no, it does happen regardless, but if you want to consider it a
feature that () for a { echo $a } ls works, then my meaning was that
it should at least print the exact arguments given.  :) ("you" in this
case being Bart, not actually you). I have nothing intelligent to say
about the rest of the mail except for nodding sagely while reading it,
so I'll elide it.

-- 
Mikael Magnusson

Re: Surprising parsing result with anonymous functions and for loops

[Peter Stephenson]

On Thu, 25 Sep 2014 15:21:43 +0200
Mikael Magnusson <mikachu@gmail.com> wrote:
> On 25 September 2014 13:39, Peter Stephenson <p.stephenson@samsung.com> wrote:
> > On Thu, 25 Sep 2014 12:02:55 +0200
> > Mikael Magnusson <mikachu@gmail.com> wrote:
> >> Okay, but does anyone at least agree that doing alias expansion at
> >> that point is highly surprising?
> >
> > You mean regardless of function behaviour?
> >
> > % alias foo=bar
> > % for i in 1; do : ; done foo
> > zsh: parse error near `bar'
> 
> Ah, when i tested this i didn't realize it always happened, since i
> tested with my ls alias (so the error said `ls' regardless). So yes
> and no, it does happen regardless, but if you want to consider it a
> feature that () for a { echo $a } ls works, then my meaning was that
> it should at least print the exact arguments given.  :) ("you" in this
> case being Bart, not actually you). I have nothing intelligent to say
> about the rest of the mail except for nodding sagely while reading it,
> so I'll elide it.

I think the change is theoretically correct anyway; we shouldn't be
looking for commands at those points, we should only be looking for
arguments, even though only special arguments like redirections are
handled here, so actually it's moot for most non-error cases.

pws

Re: Surprising parsing result with anonymous functions and for loops

[Peter Stephenson]

On Thu, 25 Sep 2014 14:52:43 +0100
Peter Stephenson <p.stephenson@samsung.com> wrote:
> On Thu, 25 Sep 2014 15:21:43 +0200
> Mikael Magnusson <mikachu@gmail.com> wrote:
> > On 25 September 2014 13:39, Peter Stephenson <p.stephenson@samsung.com> wrote:
> > > On Thu, 25 Sep 2014 12:02:55 +0200
> > > Mikael Magnusson <mikachu@gmail.com> wrote:
> > >> Okay, but does anyone at least agree that doing alias expansion at
> > >> that point is highly surprising?
> > >
> > > You mean regardless of function behaviour?
> > >
> > > % alias foo=bar
> > > % for i in 1; do : ; done foo
> > > zsh: parse error near `bar'
> 
> I think the change is theoretically correct anyway; we shouldn't be
> looking for commands at those points, we should only be looking for
> arguments, even though only special arguments like redirections are
> handled here, so actually it's moot for most non-error cases.

The only non-error case I can think of that this would effect is if you
add a non-global alias for something like "2>&1", though I can't think
of a good use for this (unlike a global alias).  You now wouldn't be
able to use that in the cases in question.  But I don't think you'd
expect to.

I'll commit this, but in case there are oddities that aren't picked up
by the tests here's a list of the cases that changed for future
reference.  "After" implies parsing for the immediately following
non-whitespace token.

for:
  - after "done"
  - after "}" in the alternative syntax
  - after "end" in the CSH syntax.

if:
  - after "fi" encountered without else ) two different cases
  - after "fi" encountered after else   )     in the code
  - after "}" encountered after else in the alternative syntax

while and repeat:
  - after "done"
  - after "}" in the alternative syntax

pws