Bug in if... then if... parsing

Bug in if... then if... parsing

[Bart Schaefer]

Reproducible at least as long ago as zsh-4.2.0.

Consider:

torch% x () {
function> if false
function if> then
function then> x=1
function then> elif true
function elif> then
function elif-then> if :
function elif-then if> else
function else> :
function else> fi
function> }
torch% 

Here it is without the PS2 prompts:

x () {
    if false
    then
        x=1
    elif true
    then
        if :
    else
        :
    fi
}

This should be a parse error --

torch% if :
if> else
zsh: parse error near `else'
torch% 

-- but instead it's accepted, the parser has somehow implied the
"missing" then/fi with a blank line between:

torch% functions -x4 x
x () {
    if false
    then
        x=1 
    elif true
    then
        if :
        then
            
        fi
    else
        :
    fi
}
torch% 

I was at one point able to get the parser into a state where this caused a
correct structure to syntax error because this weird implicit "fi" doubled
an actual "fi", but it was in the context of a larger function and I lost
it from my terminal scrollback by the time I worked out that it wasn't
something I was doing wrong.  It had something to do with "then" appearing
with other code on the same line rather than on a line by itself.

As far as I can tell this only happens when "if" immediately follows "then".
At first I thought the "elif" was also required, but no:

torch% if :
if> then
then> if :
then if> else
else> fi
torch% 

Re: Bug in if... then if... parsing

[Bart Schaefer]

On Mar 6,  2:27pm, Bart Schaefer wrote:
}
} As far as I can tell this only happens when "if" immediately follows "then".

The following seems to be the simplest fix.  I was afraid this was going
to be a lot harder to track down.

Does anyone recall why par_list() and par_list1() return a value when
literally nothing ever examines that value?  What does the 1 or 0 that
is returned even mean?

diff --git a/Src/parse.c b/Src/parse.c
index 628a9aa..349d1e4 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -1413,7 +1413,7 @@ par_if(int *cmplx)
 	}
     }
     cmdpop();
-    if (xtok == ELSE) {
+    if (xtok == ELSE || tok == ELSE) {
 	pp = ecadd(0);
 	cmdpush(CS_ELSE);
 	while (tok == SEPER)

Re: Bug in if... then if... parsing

[Peter Stephenson]

On Sun, 06 Mar 2016 15:05:46 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> Does anyone recall why par_list() and par_list1() return a value when
> literally nothing ever examines that value?  What does the 1 or 0 that
> is returned even mean?

I don't remember the details, but it was somehow related to whether you
were going to need to do more parsing.  I think tweaking the upper
levels of the parse hierarchy made it redundant.  I did run at into this
at some point last year but didn't simplify it then.

pws

diff --git a/Src/parse.c b/Src/parse.c
index 349d1e4..94ac049 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -718,7 +718,7 @@ set_sublist_code(int p, int type, int flags, int skip, int cmplx)
  */
 
 /**/
-static int
+static void
 par_list(int *cmplx)
 {
     int p, lp = -1, c;
@@ -747,19 +747,15 @@ par_list(int *cmplx)
 	    goto rec;
 	} else
 	    set_list_code(p, (Z_SYNC | Z_END), c);
-	return 1;
     } else {
 	ecused--;
-	if (lp >= 0) {
+	if (lp >= 0)
 	    ecbuf[lp] |= wc_bdata(Z_END);
-	    return 1;
-	}
-	return 0;
     }
 }
 
 /**/
-static int
+static void
 par_list1(int *cmplx)
 {
     int p = ecadd(0), c = 0;
@@ -767,11 +763,8 @@ par_list1(int *cmplx)
     if (par_sublist(&c)) {
 	set_list_code(p, (Z_SYNC | Z_END), c);
 	*cmplx |= c;
-	return 1;
-    } else {
+    } else
 	ecused--;
-	return 0;
-    }
 }
 
 /*