Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parser should point to the name of non-existent functions rather than the end of the line #2637

Closed
bgoodri opened this issue Sep 13, 2018 · 6 comments

Comments

@bgoodri
Copy link
Contributor

bgoodri commented Sep 13, 2018

Summary:

If a user tries to use a function that does not exist or otherwise specifies it wrong, the parser says something like error in ... at line X, column Y where Y is the end of the line. This confuses RStudio's error flagging (which goes off Y) and makes it look like the problem is with whatever is the last argument to the function, rather than the function itself.

Description:

See rstudio/rstudio#3474

Reproducible Steps:

Parse this

data {
  real mu;
  real<lower=0> sigma;
}
transformed data {
  real z = foo(mu, sigma);
}

Current Output:

SYNTAX ERROR, MESSAGE(S) FROM PARSER:

No matches for: 

  foo(real, real)

Function foo not found.
  error in 'model573502d9042_foo' at line 6, column 26
  -------------------------------------------------
     4: }
     5: transformed data {
     6:   real z = foo(mu, sigma);
                                 ^
     7: }
  -------------------------------------------------

Expected Output:

error in 'model573502d9042_foo' at line 6, column 14 (or maybe 12, something pointing to foo instead of the semicolon)

Additional Information:

Not a huge priority

Current Version:

v2.18.0

@bob-carpenter
Copy link
Contributor

What's wrong is that there's no signature to match the argument types. So it really is something that happens after you see the arguments. It could happen earlier if there's a way to tell if some initial arguments are inconsistent with all possibilities.

It might be possible to change the error message, but given the way it happens in the parser which is as things go along, it might not be so easy.

@VMatthijs
Copy link
Member

Stanc3 now produces

Semantic error at file "test.stan", line 6, characters 11-25:
   -------------------------------------------------
     4:  }
     5:  transformed data {
     6:    real z = foo(mu, sigma);
                    ^
     7:  }
   -------------------------------------------------

A returning function was expected but an undeclared identifier foo was supplied.

@bob-carpenter
Copy link
Contributor

bob-carpenter commented Dec 13, 2018 via email

@VMatthijs
Copy link
Member

VMatthijs commented Dec 13, 2018

Sure. Perhaps "variable" would be better? We could always specialize the message further to functions etc., but currently it is thrown elsewhere in a more generic fashion, when it's looking up an identifier in the symbol table.

If we have a model

functions {
  real foo(real[] x, vector y, int z) {return 3;}
}
data {
  real mu;
  real<lower=0> sigma;
}
transformed data {
  real z = foo(mu, sigma);
}

it says

Semantic error at file "test.stan", line 9, characters 11-25:
   -------------------------------------------------
     7:  }
     8:  transformed data {
     9:    real z = foo(mu, sigma);
                    ^
    10:  }
   -------------------------------------------------

Ill-typed arguments supplied to function foo. Available signatures:
(real[], vector, int) => real
Instead supplied arguments of incompatible type: real, real.

In case of a library function, it would list all the signatures here.

I feel like we should decide on a notation for function types for when we add them to the language (could be done swiftly once we decide on a notation) and I'd like to make the notation in error messages forward compatible with that notation. Currently, I'm writing (X, Y) => Z, but I could also imagine Fun(X, Y; Z) or (X, Y) -> Z.

@bob-carpenter
Copy link
Contributor

bob-carpenter commented Dec 13, 2018 via email

@mcol
Copy link
Contributor

mcol commented Feb 21, 2020

Fixed in stanc3. Current message is:

Semantic error in 'tmp.stan', line 6, column 11 to column 25:
   -------------------------------------------------
     4:  }
     5:  transformed data {
     6:    real z = foo(mu, sigma);
                    ^
     7:  }
   -------------------------------------------------

A returning function was expected but an undeclared identifier 'foo' was supplied.

@mcol mcol closed this as completed Feb 21, 2020
@mcol mcol modified the milestones: v3, 2.22.0 Feb 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants