stan-dev / stan

Stan development repository. The master branch contains the current release. The develop branch contains the latest stable development. See the Developer Process Wiki for details.
https://mc-stan.org
BSD 3-Clause "New" or "Revised" License
2.61k stars 369 forks source link

Keywords should be reserved in Stan #2712

Closed VMatthijs closed 4 years ago

VMatthijs commented 5 years ago

Summary:

Currently, various keywords can be used as variable names in Stan. This seems like a bad idea from a PL perspective.

Description:

For instance, lower and upper can be used as variable names.

Reproducible Steps:

The following is a valid Stan program, currently:

transformed parameters {
  real lower;
  real<lower=lower> upper;
  real<lower=upper, upper=lower> imconfusednow;
  print(imconfusednow);
}

Current Output:

No error.

Expected Output:

Syntax error on line 2.

Additional Information:

This is currently exploited in rstanarm.

Current Version:

v2.18.0

bob-carpenter commented 5 years ago

For instance, lower and upper can be used as variable names.

I agree---these should've been excluded as possible variable names.

stnorton commented 5 years ago

This is also true with i - if you declare int i and use it as a dimension in a matrix such as matrix[i,2], you received a dimension mismatch error, whereas int I works. This is potentially because it's interpreted as the iterator in a loop?

bob-carpenter commented 5 years ago

The variable i isn't a keyword in any sense. But it might be defined somewhere in your program.

@stnorton --- can you provide a Stan program where the problem you mention comes up? I didn't understand the problem you're referring to.

stnorton commented 5 years ago

@bob-carpenter I created a gist. Naming one of the dimensions i (as in the provided code) produces the following error:

Error in new_CppObject_xp(fields$.module, fields$.pointer, ...) : 
  Exception: mismatch in dimension declared and found in context; processing stage=data initialization; variable name=Z; position=0; dims declared=(8,1); dims found=(100,1)  (in 'model21b73681761c_norton_lab6' at line 8)

Changing the declaration on line 4 to int I and all the references to I fixes the error. Effectively, Stan seems to be interpreting i as being equal to 8, even though it is being passed as 100 with the data.

bob-carpenter commented 5 years ago

Thanks so much for the reproducible example. I verified it's a bug in 2.18.

I'm guessing there's some internal use of the variable i that's conflicting. We try to avoid that, but something must've slipped into the generated code. I'm going to rename and edit the issue to make it clearer to the devs what is going to need to be fixed. This may already be fixed in 2.19, in fact, as a lot of the code generation changed there.

bob-carpenter commented 5 years ago

I verified this is fixed in 2.19 as a pleasant side effect of rebuilding the generator.