Re: Parse error (lack thereof) on incomplete loops

[Peter Stephenson <p.stephenson@xxxxxxxxxxx>]

On Thu, 2018-10-04 at 08:49 -0700, Bart Schaefer wrote:
> This went by in a zsh-users thread so it may have been missed:

Not sure who the additional audience is...

> % { while false
> cursh while> }
> %
> 
> This indicates a parsing problem in 5.6, because in 5.0:
> 
> % { while false
> cursh while> }
> zsh: parse error near `}'

It affects for, repeat, until and if, too, and isn't hard to turn back
into a parse error.  Patch attached, though I'm not going to apply it
at
the moment.

However, I'm very far from convinced this is actually a bug.  If you
turn off SHORT_LOOPS you'll find you get the error back; the
alternative loop syntax is defined in such a woolly fashion perhaps you
actually can get away with this.  The short-loop user is probably
primed
to notice such things as that there's no following command so it'll
just
do nothing.

I'm therefore inclined to leave it alone since the SHORT_LOOPS user
(Marc) is perfectly happy with it and a lot of us avoid the alternative
syntax like the plague.

diff --git a/Src/parse.c b/Src/parse.c
index 83383f1..a96cdcb 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -446,12 +446,17 @@ ecstrcode(char *s)
         par_list(C); \
         if (eu == ecused) ecadd(WCB_END()); \
     } while (0)
-#define par_save_list1(C) \
-    do { \
-        int eu = ecused; \
-        par_list1(C); \
-        if (eu == ecused) ecadd(WCB_END()); \
-    } while (0)
+
+/**/
+static int
+par_save_list1(int *cmplx)
+{
+    int eu = ecused;
+    int ret = par_list1(cmplx);
+    if (eu == ecused)
+	ecadd(WCB_END());
+    return ret;
+}
 
 
 /**/
@@ -766,7 +771,7 @@ par_list(int *cmplx)
 }
 
 /**/
-static void
+static int
 par_list1(int *cmplx)
 {
     int p = ecadd(0), c = 0;
@@ -774,8 +779,11 @@ par_list1(int *cmplx)
     if (par_sublist(&c)) {
 	set_list_code(p, (Z_SYNC | Z_END), c);
 	*cmplx |= c;
-    } else
+	return 1;
+    } else {
 	ecused--;
+	return 0;
+    }
 }
 
 /*
@@ -1151,8 +1159,8 @@ par_for(int *cmplx)
 	zshlex();
     } else if (unset(SHORTLOOPS)) {
 	YYERRORV(oecused);
-    } else
-	par_save_list1(cmplx);
+    } else if (!par_save_list1(cmplx))
+	YYERRORV(oecused);
 
     ecbuf[p] = (sel ?
 		WCB_SELECT(type, ecused - 1 - p) :
@@ -1439,7 +1447,8 @@ par_if(int *cmplx)
 	} else {
 	    cmdpop();
 	    cmdpush(nc);
-	    par_save_list1(cmplx);
+	    if (!par_save_list1(cmplx))
+		YYERRORV(oecused);
 	    ecbuf[pp] = WCB_IF(type, ecused - 1 - pp);
 	    incmdpos = 1;
 	    break;
@@ -1512,8 +1521,8 @@ par_while(int *cmplx)
 	zshlex();
     } else if (unset(SHORTLOOPS)) {
 	YYERRORV(oecused);
-    } else
-	par_save_list1(cmplx);
+    } else if (!par_save_list1(cmplx))
+	YYERRORV(oecused);
 
     ecbuf[p] = WCB_WHILE(type, ecused - 1 - p);
 }
@@ -1561,8 +1570,8 @@ par_repeat(int *cmplx)
 	zshlex();
     } else if (unset(SHORTLOOPS)) {
 	YYERRORV(oecused);
-    } else
-	par_save_list1(cmplx);
+    } else if (!par_save_list1(cmplx))
+	YYERRORV(oecused);
 
     ecbuf[p] = WCB_REPEAT(ecused - 1 - p);
 }