PATCH (!): Re: PATCH: (very) bad syntax error checking

["Bart Schaefer" <schaefer@xxxxxxxxxxxxxxxxxxxxxxx>]

On May 26,  3:24pm, Peter Stephenson wrote:
} Subject: PATCH: (very) bad syntax error checking
}
} It's rather late in the day to come across problems like this.
} 
} % zsh -f ./test.zsh
} ./test.zsh:3: parse error near `fi'
} This line should not be executed.

This was supposedly fixed, by PWS, in zsh-workers/4191, before the 3.1.5
release; though Zefram changed it slightly, according to the ChangeLog.

As it happens, one of the customizations I have in my local copy of 3.1.6
is the patch from zsh-workers/4196, which I've been stubbornly hanging on
to ever since Zefram chose not to include it in 3.1.5, so I tried it with
ARGV0=ksh:

zagzig[274] Src/zsh -f /tmp/foo/test.zsh
/tmp/foo/test.zsh:3: parse error near `fi'
This line should not be executed.
zagzig[275] ARGV0=ksh Src/zsh -f /tmp/foo/test.zsh
/tmp/foo/test.zsh:3: parse error near `fi'
zagzig[276] 

That would seem to mean the problem is with the value of `noerrexit' around
line 123 of init.c, but in fact that's NOT the case.  Here again is the
shell code in question:

    if true; # then missing
      print hello
    fi

Remove the first two lines; then:

zagzig[277] Src/zsh -f /tmp/foo/test.zsh
/tmp/foo/test.zsh:1: parse error near `fi'
This line should not be executed.
zagzig[278] ARGV0=ksh Src/zsh -f /tmp/foo/test.zsh
/tmp/foo/test.zsh:1: parse error near `fi'
This line should not be executed.

So the problem is twofold; first, there appears to be some very old cruft
in par_if() in parse.c left over from when the SHORT_LOOPS option affected
the bodies of "if" statements.  This causes the ksh/zsh difference I first
noticed.  Unfortunately, I don't know how to remove it without breaking
still-valid csh-compat syntax like `if (true) print hello'.  It probably
doesn't need to be removed after fixing the next problem.

Second, encountering the token "fi" alone on a line is not setting the
returned token to LEXERR, so in init.c the test of (tok == LEXERR) fails
(the value of `tok' is, not surpisingly, `FI').

What follows appears to fix it, but Sven should confirm.  For one thing,
I wonder why parse_list() doesn't use YYERROR() ... is there some reason
why `ecused' should not be set to 0 in that specific case?

I included a hunk for the grammar test, but why is the line number `-1'?

(The init.c hunk is just some whitespace cleanup.)

Index: Src/parse.c
===================================================================
@@ -459,6 +459,7 @@
 	}
     }
     if (!r) {
+	tok = LEXERR;
 	if (errflag) {
 	    yyerror(0);
 	    ecused--;
@@ -491,10 +492,8 @@
     yylex();
     init_parse();
     par_list(&c);
-#if 0 
-   if (tok == LEXERR)
-#endif
-   if (tok != ENDINPUT) {
+    if (tok != ENDINPUT) {
+	tok = LEXERR;
 	yyerror(0);
 	return NULL;
     }
Index: Src/init.c
===================================================================
@@ -944,7 +944,7 @@
 	execode(prog, 1, 0);
 	popheap();
     } else
-	loop(0, 0);		     /* loop through the file to be sourced        */
+	loop(0, 0);		     /* loop through the file to be sourced  */
     sourcelevel--;
 
     /* restore the current shell state */
Index: Test/01grammar.ztst
===================================================================
@@ -105,6 +105,11 @@
   fi
 0:`if ...' (iii)
 >false
+  if true;
+    :
+  fi
+1d:`if ...' (iv)
+?ZTST_execchunk:-1: parse error near `fi'
 
   for name in word to term; do
     print $name

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com