Double ;; Induces Parser Error

alashworth commented 5 years ago

Issue by betanalpha Sunday Aug 13, 2017 at 22:09 GMT Originally opened as https://github.com/stan-dev/stan/issues/2379

Description:

Extra semicolons ending a line throw a parser error despite the fact that an empty line causes no problems.

Reproducible Steps:

parameters {
  real x1;;
  real x2;
}

model {
  x1 ~ normal(0, 1);
  x2 ~ normal(0, 1);
}

Current Output:

SYNTAX ERROR, MESSAGE(S) FROM PARSER:

  error in 'bad.stan' at line 2, column 11
  -------------------------------------------------
     1: parameters {
     2:   real x1;;
                  ^
     3:   real x2;
  -------------------------------------------------

PARSER EXPECTED: <one of the following:
  a variable declaration, beginning with type,
      (int, real, vector, row_vector, matrix, unit_vector,
       simplex, ordered, positive_ordered,
       corr_matrix, cov_matrix,
       cholesky_corr, cholesky_cov
  or '}' to close variable declarations>

Expected Output:

No error or something more explicit like "Double semicolons not allowed".

Current Version:

v2.16.0

alashworth commented 5 years ago

Comment by bob-carpenter Monday Aug 14, 2017 at 11:31 GMT

The behavior you cite is intentional and the error message is correct.

The parameters block consists of a sequence of variable declarations. A single semicolon (;) doesn't constitute a variable declaration.

On the other hand, if you look at contexts that allow statements, you'll find double semicolons are OK because ; is a well-formed statement.

transformed data {
  real x;
  x = 10; ;
}

Obviously if you filed this as a bug report, you think the behavior should be changed.

I see two ways that could work.

we change the grammar and AST to allow empty variable declarations and make sure all the generator code, etc., doesn't generate anything when it sees them.
we somehow sneak a lit(";") into the sequence of variable declarations in a way that doesn't do anything to the semantics.

I don't like (1) because I don't think there should be empty declarations. With (2), the question is how to finagle the grammar. I'm thinking it'd have to be something like this:

variable_declarations ::= -lit(";") << variable_declarations
                          | ...

I think the type of the lit will just disappear, but we'll need to check that.

alashworth commented 5 years ago

Comment by betanalpha Monday Aug 14, 2017 at 13:53 GMT

I am very happy keeping the current behavior as a parser error, but then I would recommend a better error message. The problem with the current message is that the naive user (i.e. me) might have their eye drawn to the well-posed line and then get frustrated at not understanding why it's not working. Can we explicitly check for empty variable declaration and give a more explicit message, or even just add "whitespace lines with no variable declaration are also not allowed" in the existing message?

alashworth / test-issue-import