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

Misleading error message when function multiply defined #2526

Closed
bob-carpenter opened this issue May 25, 2018 · 6 comments
Closed

Misleading error message when function multiply defined #2526

bob-carpenter opened this issue May 25, 2018 · 6 comments
Assignees

Comments

@bob-carpenter
Copy link
Member

From @ksvanhorn on May 25, 2018 16:47

Summary:

When a function is defined twice, the error message points to the function AFTER the second definition.

Description:

A program defines the function "rot_matrix" twice in the "functions" block. The error message points at the beginning of the definition of a later function, "scale_matrix".

Reproducible Steps:

Run stanc on the following program:

functions {
  matrix rot_matrix() {
    return rep_matrix(0,1,1);
  }
  
  matrix rot_matrix() {
    return rep_matrix(0,1,1);
  }
  
  matrix scale_matrix() {
    return rep_matrix(0,1,1);
  }
}
data {
  int<lower = 1> N;
  vector[N] y;
}
parameters {
  real<lower=0> sigma;
}
model {
  sigma ~ normal(0, 1);
  y ~ normal(0, sigma);
}     

Current Output:

SYNTAX ERROR, MESSAGE(S) FROM PARSER:

Parse Error.  Function already defined, name=rot_matrix  error in 'model661b1902a01a_foo' at line 10, column 3
  -------------------------------------------------
     8:   }
     9:   
    10:   matrix scale_matrix() {
          ^
    11:     return rep_matrix(0,1,1);
  -------------------------------------------------
 
Error in stanc(file = file, model_code = model_code, model_name = model_name,  : 
  failed to parse Stan model 'foo' due to the above error. 

Expected Output:

The caret should point at the beginning of the second definition of rot_matrix.

RStan Version:

2.17.3

R Version:

R version 3.4.3 (2017-11-30)

Operating System:

OS X 10.13.4

Copied from original issue: stan-dev/rstan#533

@bob-carpenter
Copy link
Member Author

From @bgoodri on May 25, 2018 18:1

I agree this is misleading, but I am sure it applies to all interfaces so
the issue should get moved to the stan-dev/stan repo.

On Fri, May 25, 2018 at 12:47 PM, Kevin S. Van Horn <
[email protected]> wrote:

Summary:

When a function is defined twice, the error message points to the function
AFTER the second definition.
Description:

A program defines the function "rot_matrix" twice in the "functions"
block. The error message points at the beginning of the definition of a
later function, "scale_matrix".
Reproducible Steps:

Run stanc on the following program:

functions {
matrix rot_matrix() {
return rep_matrix(0,1,1);
}

matrix rot_matrix() {
return rep_matrix(0,1,1);
}

matrix scale_matrix() {
return rep_matrix(0,1,1);
}
}
data {
int<lower = 1> N;
vector[N] y;
}
parameters {
real<lower=0> sigma;
}
model {
sigma ~ normal(0, 1);
y ~ normal(0, sigma);
}

Current Output:

SYNTAX ERROR, MESSAGE(S) FROM PARSER:

Parse Error. Function already defined, name=rot_matrix error in 'model661b1902a01a_foo' at line 10, column 3

 8:   }
 9:
10:   matrix scale_matrix() {
      ^
11:     return rep_matrix(0,1,1);

Error in stanc(file = file, model_code = model_code, model_name = model_name, :
failed to parse Stan model 'foo' due to the above error.

Expected Output:

The caret should point at the beginning of the second definition of
rot_matrix.
RStan Version:

2.17.3
R Version:

R version 3.4.3 (2017-11-30)
Operating System:

OS X 10.13.4


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
stan-dev/rstan#533, or mute the thread
https://github.com/notifications/unsubscribe-auth/ADOrqkl4V3uhyIUUO4kyUyDUCvU6T24sks5t2DWPgaJpZM4UOR-Z
.

@VMatthijs
Copy link
Member

VMatthijs commented Dec 13, 2018

stanc3 currently says

Semantic error at file "test.stan", line 6, characters 9-19:
   -------------------------------------------------
     4:    }
     5:
     6:    matrix rot_matrix() {
                  ^
     7:      return rep_matrix(0,1,1);
     8:    }
   -------------------------------------------------

Identifier  rot_matrix  is already in use.

stanc2 still says

SYNTAX ERROR, MESSAGE(S) FROM PARSER:
Parse Error.  Function already defined, name=rot_matrix error in 'stan/src/test/test-models/good/empty.stan' at line 10, column 3
  -------------------------------------------------
     8:   }
     9:
    10:   matrix scale_matrix() {
          ^
    11:     return rep_matrix(0,1,1);
  -------------------------------------------------

@bob-carpenter
Copy link
Member Author

This is great. I don't like the extra spaces around rot_matrix --- I think identifiers should be quoted as in C++ compiler error messages. In general, I'd follow C++ error messages because they've had a long time to think about these and get them right.

Here, I'd prefer to see something like " 'rot_matrix' is already defined".

@bob-carpenter
Copy link
Member Author

In C++, this program

int foo(int x) { return x; }
int foo(int x) { return x; }
int main() { return 1; }

produces

cpperr.cpp:2:5: error: redefinition of 'foo'
int foo(int x) { return x; }
    ^
cpperr.cpp:1:5: note: previous definition is here
int foo(int x) { return x; }
    ^
1 error generated.

I like that it says "redefinition" instead of "already in use".

@VMatthijs
Copy link
Member

This is great. I don't like the extra spaces around rot_matrix --- I think identifiers should be quoted as in C++ compiler error messages. In general, I'd follow C++ error messages because they've had a long time to think about these and get them right.

Just fixed this.

@mcol
Copy link
Contributor

mcol commented Feb 9, 2020

This was fixed in stanc3.

@mcol mcol closed this as completed Feb 9, 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

5 participants