[Help-smalltalk] [PATCH] Improve error recovery

[Paolo Bonzini]

This patch improves error recovery so that one or two new-lines 
(depending on where the error happens) will make the parser functional 
again without forgetting the bindings:

st> a := 5
5
st> b:
stdin:13: expected expression
st> a
5


A bang will forget the bindings as usual:

st> a := 5
5
st> b: !
stdin:4: expected expression
st> a
nil

Thanks to Tony for subliminally convincing me that current error 
recovery was bad and undocumented.

Paolo

* looking for address@hidden/smalltalk--devo--2.2--patch-683 to compare with
* comparing to address@hidden/smalltalk--devo--2.2--patch-683
M  libgst/gst-parse.c
M  libgst/gst-parse.h
M  libgst/comp.c
M  libgst/lex.c
M  libgst/lex.h
M  libgst/comp.h

* modified files

--- orig/libgst/comp.c
+++ mod/libgst/comp.c
@@ -113,12 +113,6 @@ mst_Boolean _gst_skip_compilation = fals
    Most often, the caller does not care about the returned value,
    since it often is called from a radically different context.  */
 OOP _gst_last_returned_value = NULL;
-
-/* This is set to true by the parser or the compiler if an error
-   (respectively, a parse error or a semantic error) is found, and
-   avoids that _gst_execute_statements tries to execute the result of
-   the compilation.  */
-mst_Boolean _gst_had_error = false;
 
 
 


--- orig/libgst/comp.h
+++ mod/libgst/comp.h
@@ -253,13 +253,6 @@ extern int _gst_declare_tracing 
 extern mst_Boolean _gst_skip_compilation 
   ATTRIBUTE_HIDDEN;
 
-/* This is set to true by the parser or the compiler if an error
-   (respectively, a parse error or a semantic error) is found, and
-   avoids that _gst_execute_statements tries to execute the result of
-   the compilation.  */
-extern mst_Boolean _gst_had_error 
-  ATTRIBUTE_HIDDEN;
-
 /* This holds whether the compiler should make the compiled methods
    untrusted.  */
 extern mst_Boolean _gst_untrusted_methods 


--- orig/libgst/gst-parse.c
+++ mod/libgst/gst-parse.c
@@ -349,6 +349,9 @@ parse_chunks (gst_parser *p) 
   else
     {
       OOP oldTemporaries = _gst_push_temporaries_dictionary ();
+      jmp_buf old_recover;
+      memcpy (old_recover, p->recover, sizeof (p->recover));
+      setjmp (p->recover);
       while (token (p, 0) != EOF && token (p, 0) != '!')
         {
           /* Pick the production here, so that subsequent 
@@ -361,6 +364,7 @@ parse_chunks (gst_parser *p) 
 
       lex_skip_if (p, '!', false);
       _gst_pop_temporaries_dictionary (oldTemporaries);
+      memcpy (p->recover, old_recover, sizeof (p->recover));
     }
 }
 
@@ -393,7 +397,6 @@ expected (gst_parser *p, int token, ...)
       token = va_arg (ap, int);
     }
 
-#define TOKEN_SEP
 #define TOKEN_DEF(name, val, str, subsume)                             \
   if ((named_tokens & (1 << (val - FIRST_TOKEN))) != 0                 \
       && (subsume == -1                                                        
\
@@ -405,7 +408,6 @@ expected (gst_parser *p, int token, ...)
 
   TOKEN_DEFS
 #undef TOKEN_DEF
-#undef TOKEN_SEP
 
     msg = fildelete (out_fil);
   _gst_errorf ("%s", msg);
@@ -420,19 +422,18 @@ static void
 recover_error (gst_parser *p)
 {
   if (p->state != PARSE_METHOD)
-    for (;;)
-      {
-       /* Find the final bang.  */
-       if (token (p, 0) == EOF)
-         break;
-       if (token (p, 0) == '!')
-         {
-           _gst_free_tree ();
-           lex (p);
-           break;
-         }
+    {
+      _gst_error_recovery = true;
+
+      /* Find the final bang or, if in the REPL, a newline.  */
+      while (token (p, 0) != EOF
+            && token (p, 0) != '!'
+            && token (p, 0) != ERROR_RECOVERY)
        lex (p);
-      }
+
+      _gst_error_recovery = false;
+      lex_skip_if (p, ERROR_RECOVERY, false);
+    }
 
   longjmp (p->recover, 1);
 }
@@ -568,12 +569,17 @@ parse_scoped_definition (gst_parser *p, 
 static void
 parse_eval_definition (gst_parser *p)
 {
-  tree_node tmps = NULL, stmts;
-  tree_node first_stmt = NULL;
+  tree_node tmps = NULL, stmts = NULL;
   OOP oldDictionary = _gst_push_temporaries_dictionary ();
+  jmp_buf old_recover;
+
+  memcpy (old_recover, p->recover, sizeof (p->recover));
+  if (setjmp (p->recover) == 0)
+    {
+      tmps = parse_temporaries (p, false);
+      stmts = parse_statements (p, NULL, true);
+    }
 
-  tmps = parse_temporaries (p, false);
-  stmts = parse_statements (p, first_stmt, true);
   if (stmts && !_gst_had_error)
     {
       if (_gst_regression_testing)
@@ -584,11 +590,14 @@ parse_eval_definition (gst_parser *p)
 
       if (_gst_regression_testing && !_gst_had_error)
         printf ("returned value is %O\n", _gst_last_returned_value);
+      _gst_had_error = false;
     }
 
-  _gst_pop_temporaries_dictionary (oldDictionary);
   _gst_free_tree ();
-  _gst_had_error = false;
+  _gst_pop_temporaries_dictionary (oldDictionary);
+  memcpy (p->recover, old_recover, sizeof (p->recover));
+  if (_gst_had_error)
+    longjmp (p->recover, 1);
 }
 
 static mst_Boolean


--- orig/libgst/gst-parse.h
+++ mod/libgst/gst-parse.h
@@ -53,35 +53,35 @@
 #define GST_PARSE_H
 
 #define TOKEN_DEFS \
-  TOKEN_DEF (SCOPE_SEPARATOR, 261, "'.' or '::'", -1) TOKEN_SEP \
-  TOKEN_DEF (ASSIGNMENT, 262, "'_' or ':='", -1) TOKEN_SEP \
-  TOKEN_DEF (SHEBANG, 263, "'#!'", -1) TOKEN_SEP \
-  TOKEN_DEF (IDENTIFIER, 264, "identifier", -1) TOKEN_SEP \
-  TOKEN_DEF (BINOP, 265, "binary operator", -1) TOKEN_SEP \
-  TOKEN_DEF (KEYWORD, 266, "keyword", -1) TOKEN_SEP \
-  TOKEN_DEF (STRING_LITERAL, 267, "string literal", -1) TOKEN_SEP \
-  TOKEN_DEF (SYMBOL_LITERAL, 268, "symbol literal", -1) TOKEN_SEP \
-  TOKEN_DEF (INTEGER_LITERAL, 269, "integer literal", -1) TOKEN_SEP \
-  TOKEN_DEF (LARGE_INTEGER_LITERAL, 270, "integer literal", 269) TOKEN_SEP \
-  TOKEN_DEF (BYTE_LITERAL, 271, "small integer literal", 269) TOKEN_SEP \
-  TOKEN_DEF (FLOATD_LITERAL, 272, "floating-point literal", -1) TOKEN_SEP \
-  TOKEN_DEF (FLOATE_LITERAL, 273, "floating-point literal", 272) TOKEN_SEP \
-  TOKEN_DEF (FLOATQ_LITERAL, 274, "floating-point literal", 272) TOKEN_SEP \
-  TOKEN_DEF (SCALED_DECIMAL_LITERAL, 275, "decimal literal", -1) TOKEN_SEP \
-  TOKEN_DEF (CHAR_LITERAL, 276, "character literal", -1)
+  TOKEN_DEF (SCOPE_SEPARATOR, 261, "'.' or '::'", -1) \
+  TOKEN_DEF (ASSIGNMENT, 262, "'_' or ':='", -1) \
+  TOKEN_DEF (SHEBANG, 263, "'#!'", -1) \
+  TOKEN_DEF (IDENTIFIER, 264, "identifier", -1) \
+  TOKEN_DEF (BINOP, 265, "binary operator", -1) \
+  TOKEN_DEF (KEYWORD, 266, "keyword", -1) \
+  TOKEN_DEF (STRING_LITERAL, 267, "string literal", -1) \
+  TOKEN_DEF (SYMBOL_LITERAL, 268, "symbol literal", -1) \
+  TOKEN_DEF (INTEGER_LITERAL, 269, "integer literal", -1) \
+  TOKEN_DEF (LARGE_INTEGER_LITERAL, 270, "integer literal", 269) \
+  TOKEN_DEF (BYTE_LITERAL, 271, "small integer literal", 269) \
+  TOKEN_DEF (FLOATD_LITERAL, 272, "floating-point literal", -1) \
+  TOKEN_DEF (FLOATE_LITERAL, 273, "floating-point literal", 272) \
+  TOKEN_DEF (FLOATQ_LITERAL, 274, "floating-point literal", 272) \
+  TOKEN_DEF (SCALED_DECIMAL_LITERAL, 275, "decimal literal", -1) \
+  TOKEN_DEF (CHAR_LITERAL, 276, "character literal", -1) \
+  TOKEN_DEF (ERROR_RECOVERY, 277, "newline", -1)
 
 #define FIRST_TOKEN (SCOPE_SEPARATOR)
 #define NUM_TOKENS (CHAR_LITERAL - SCOPE_SEPARATOR + 1)
 
-#define TOKEN_SEP ,
 #define TOKEN_DEF(name, val, str, subsume) \
-  name = val
+  name = val,
 
 enum yytokentype {
   TOKEN_DEFS
+  FIRST_UNUSED_TOKEN
 };
 
-#undef TOKEN_SEP
 #undef TOKEN_DEF
 
 typedef union YYSTYPE {


--- orig/libgst/lex.c
+++ mod/libgst/lex.c
@@ -84,6 +84,16 @@ struct obstack *_gst_compilation_obstack
    code.  */
 mst_Boolean _gst_report_errors = true;
 
+/* This is set to true by the parser or the compiler if an error
+   (respectively, a parse error or a semantic error) is found, and
+   avoids that _gst_execute_statements tries to execute the result of
+   the compilation.  */
+mst_Boolean _gst_had_error = false;
+
+/* This is set to true by the parser if error recovery is going on.
+   In this case ERROR_RECOVERY tokens are generated.  */
+mst_Boolean _gst_error_recovery = false;
+
 /* The location of the first error reported, stored here so that
    compilation primitives can pass them to Smalltalk code.  */
 char *_gst_first_error_str = NULL;
@@ -460,16 +470,22 @@ int
 scan_newline (int c,
              YYSTYPE * lvalp)
 {
-  if (_gst_get_cur_stream_prompt ()
-      && parenthesis_depth == 0
-      && last_token != 0
-      && last_token != '.' && last_token != '!' && last_token != KEYWORD
-      && last_token != BINOP && last_token != '|' && last_token != '<'
-      && last_token != '>' && last_token != ';'
-      && last_token != ASSIGNMENT && last_token != SCOPE_SEPARATOR)
-    return ('.');
-  else
-    return 0;
+  if (_gst_get_cur_stream_prompt ())
+    {
+      /* Newline is special-cased in the REPL.  */
+      if (_gst_error_recovery)
+        return ERROR_RECOVERY;
+
+      if (parenthesis_depth == 0
+          && last_token != 0
+          && last_token != '.' && last_token != '!' && last_token != KEYWORD
+          && last_token != BINOP && last_token != '|' && last_token != '<'
+          && last_token != '>' && last_token != ';'
+          && last_token != ASSIGNMENT && last_token != SCOPE_SEPARATOR)
+        return ('.');
+    }
+
+  return 0;
 }
 
 


--- orig/libgst/lex.h
+++ mod/libgst/lex.h
@@ -61,6 +61,18 @@
 extern mst_Boolean _gst_report_errors 
   ATTRIBUTE_HIDDEN;
 
+/* This is set to true by the parser or the compiler if an error
+   (respectively, a parse error or a semantic error) is found, and
+   avoids that _gst_execute_statements tries to execute the result of
+   the compilation.  */
+extern mst_Boolean _gst_had_error
+  ATTRIBUTE_HIDDEN;
+
+/* This is set to true by the parser if error recovery is going on.
+   In this case ERROR_RECOVERY tokens are generated.  */
+extern mst_Boolean _gst_error_recovery
+  ATTRIBUTE_HIDDEN;
+
 /* The location of the first error reported, stored here so that
    compilation primitives can pass them to Smalltalk code.  */
 extern char *_gst_first_error_str