Merge pull request #1587 from stan-dev/improve-parser-error-fn-types

Make the syntax error when declaring a function with constrained arguments more specific

Author: WardBrian

src/frontend/Parse.ml

@@ -11,7 +11,7 @@ let drive_parser parse_fun =
       (Preprocessor.current_buffer ())
       () in
   let success prog = {prog with Ast.comments= Preprocessor.get_comments ()} in
-  let failure error_state =
+  let failure prev error_state =
     (* see the Menhir manual for the description of error messages support *)
     let env =
       match error_state with
@@ -34,11 +34,19 @@ let drive_parser parse_fun =
           [%message
             "Failed to find error for parser error state " (state : int)] in
     let location =
+      let env =
+        match prev with
+        (* if we errored because of one of our UNREACHABLE token, use the
+           location immediately preceding for a more informative error *)
+        | Interp.InputNeeded prev_env
+          when Interp.acceptable prev Parser.UNREACHABLE Lexing.dummy_pos ->
+            prev_env
+        | _ -> env in
       Preprocessor.location_span_of_positions (Interp.positions env) in
     Syntax_error.parse_error message location in
   let startp = (Preprocessor.current_buffer ()).lex_curr_p in
   Syntax_error.try_with (fun () ->
-      Interp.loop_handle success failure input (parse_fun startp))
+      Interp.loop_handle_undo success failure input (parse_fun startp))
 
 let to_lexbuf file_or_code =
   match file_or_code with

src/frontend/parser.messages

@@ -267,6 +267,33 @@ reserved_word:
   | UPPER { "upper", $loc, false }
   | ARRAY { "array", $loc, true }
 
+(* useful for generating better messages in locations where these are not permitted *)
+%inline no_constraints:
+  | basic_type LABRACK UNREACHABLE
+    { () }
+  | constrained_vector UNREACHABLE
+    { () }
+  | constrained_matrix UNREACHABLE
+    { () }
+
+constrained_vector:
+  | ORDERED
+  | POSITIVEORDERED
+  | SIMPLEX
+  | UNITVECTOR
+  | SUMTOZEROVEC
+    { () }
+
+constrained_matrix:
+  | CHOLESKYFACTORCORR
+  | CHOLESKYFACTORCOV
+  | CORRMATRIX
+  | COVMATRIX
+  | SUMTOZEROMAT
+  | STOCHASTICCOLUMNMATRIX
+  | STOCHASTICROWMATRIX
+    { () }
+
 function_def:
   | rt=return_type name=decl_identifier LPAREN args=separated_list(COMMA, arg_decl)
     RPAREN b=statement
@@ -298,16 +325,11 @@ unsized_type:
   (* This is just a helper for the fact that we don't support the old array syntax any more
     It can go away at some point if it starts causing conflicts.
   *)
-  | bt=basic_type dims=unsized_dims {
-    parse_error
-      (Fmt.str
-         "%@{<light_red>Ill-formed type.%@} %@{<yellow>It looks like you are \
-          trying to use the old array syntax.@ Please use the new syntax:%@}@ \
-          @[<h>array[%s] %a@]@\n"
-         (String.make (dims - 1) ',')
-         UnsizedType.pp bt)
-      $loc(dims)
-  }
+  | basic_type LBRACK UNREACHABLE
+    { (* This code will never be reached *)
+       Common.ICE.internal_compiler_error
+          [%message "the UNREACHABLE token should never be produced"]
+    }
   | bt=basic_type
     {  grammar_logger "unsized_type";
        bt
@@ -339,6 +361,11 @@ basic_type:
     {  grammar_logger "basic_type COMPLEXROWVECTOR" ; UnsizedType.UComplexRowVector }
   | COMPLEXMATRIX
     {  grammar_logger "basic_type COMPLEXMATRIX" ; UnsizedType.UComplexMatrix }
+  | no_constraints
+    { (* This code will never be reached. The parser state exists to make the error more specific. *)
+       Common.ICE.internal_compiler_error
+          [%message "the UNREACHABLE token should never be produced"]
+    }
 
 unsized_dims:
   | LBRACK cs=list(COMMA) RBRACK
@@ -386,13 +413,17 @@ decl(type_rule, rhs):
     let ty = (ty, trans) in
     parse_error
       (Fmt.str
-         "%@{<light_red>Ill-formed declaration.%@} %@{<green>\";\"%@} expected \
-          after variable declaration.@ %@{<yellow>It looks like you are trying \
+         "@[<v 2>%@{<light_red>Ill-formed declaration.%@} %@{<green>\";\"%@} expected \
+          after variable declaration.@ %@{<i>It looks like you are trying \
           to use the old array syntax.@ Please use the new syntax:%@}@ @[<h>%a \
-          %s;@]@\n"
+          %s;@]@]@\n"
          Pretty_printing.pp_transformed_type ty id.name)
       $loc(dims)
     }
+  | DATABLOCK UNREACHABLE {
+    Common.ICE.internal_compiler_error
+      [%message "the UNREACHABLE token should never be produced"]
+  }
   | ty=higher_type(type_rule)
     (* additional indirection only for better error messaging *)
     v = id_and_optional_assignment(rhs, decl_identifier) vs=option(remaining_declarations(rhs)) SEMICOLON
@@ -481,6 +512,11 @@ sized_basic_type:
     { grammar_logger "COMPLEXROWVECTOR_var_type" ; (SizedType.SComplexRowVector e , Identity) }
   | COMPLEXMATRIX LBRACK e1=expression COMMA e2=expression RBRACK
     { grammar_logger "COMPLEXMATRIX_var_type" ; (SizedType.SComplexMatrix (e1, e2), Transformation.Identity) }
+  | no_constraints
+    { (* This code will never be reached. The parser state exists to make the error more specific. *)
+       Common.ICE.internal_compiler_error
+          [%message "the UNREACHABLE token should never be produced"]
+    }
 
 top_var_type:
   | INT r=range_constraint

src/frontend/parser.mly

@@ -0,0 +1,4 @@
+transformed parameters {
+
+}
+mdel {}

test/integration/bad/err-expected-end-of-tp.stan

@@ -0,0 +1,3 @@
+array[,,] x() {
+    return ones_array(3, 3);
+}

test/integration/bad/lang/bad4.stanfunctions

@@ -0,0 +1,3 @@
+model {
+  array[3,3] x;
+}

test/integration/bad/lang/bad_decl5.stan

@@ -0,0 +1,3 @@
+model {
+  array real x;
+}

test/integration/bad/lang/bad_decl6.stan

@@ -0,0 +1,3 @@
+transformed parameters {
+    jacobian += while;
+}

test/integration/bad/lang/bad_jacobian.stan

@@ -0,0 +1,3 @@
+transformed data {
+  vector[2] foo = [ ;
+}

test/integration/bad/lang/incomplete10.stan

@@ -0,0 +1,3 @@
+model {
+  real; foo;
+}