Skip to content

Commit

Permalink
Prevent user from declaring variables and functions with same name
Browse files Browse the repository at this point in the history
* Today use can "declarae" a variable in NEURON block and
  then PROCEDURE or FUNCTION with the same name. For example,
  below doesn't raise any error today:

   ```python
   NEURON {
       SUFFIX glia
       RANGE alfa
   }

   FUNCTION alfa(v(mV)) {
       alfa = exp(v)
   }
   ```

* The reason this works is that the `RANGE alfa` declaration in
  NEURON block id not complete (i.e. variable needs to be "defined"
  in block like ASSIGNED, PARAMETER etc) and hence doesn't appear
  anywhere in the generated code. If we try to use `alfa` as variable
  then that is warning and resulted into compilation error.

* This behaviour kind of works but error prone. Apart from corner cases
  and confusing behaviour, it's difficult to do certain analysis that
  we would like to do (e.g. in new NMODL code generation).

* So for the robustness and simplicity, we now throw an error if user
  try to declarte variable and function with the same name.

  ```console
   $ ./bin/nocmodl mod_x/test.mod
       Translating mod_x/test.mod into mod_x/test.cpp
       Error: alfa used as both variable and function in file mod/test.mod
  ```
  • Loading branch information
pramodk committed Sep 23, 2022
1 parent d2564ff commit 7c4660b
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 19 deletions.
36 changes: 34 additions & 2 deletions src/nmodl/consist.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,30 @@ extern Symbol* indepsym;
err = 1; \
}

/**
* Check if variable declared in NEURON block is conflicting with a PROCEDURE or FUNCTION
*
* In order to avoid "declaring" variable in NEURON block and then defining
* PROCEDURE or FUNCTION with the same name, we check if the variable is
* function and has one of the NRN* type.
*/
int is_var_declared_as_function(Symbol* s) {
int type = s->nrntype;
int usage = s->usage;

// if not function or procedure name then return already
if (! (usage & FUNCT) ) {
return 0;
}

// conflicting name if presence of NEURON block variable type
return ( type & NRNRANGE
|| type & NRNGLOBAL
|| type & NRNPOINTER
|| type & NRNBBCOREPOINTER
|| type & NRNEXTRN);
}

void consistency() {
Symbol* s;
Item* qs;
Expand Down Expand Up @@ -51,10 +75,18 @@ void consistency() {
con("STEPPED", STEP1, 0);
con("CONSTANT UNITS FACTOR", UNITDEF, 0);
tu = s->usage;
if ((tu & DEP) && (tu & FUNCT))
if ((tu & DEP) && (tu & FUNCT)) {
diag(s->name, " used as both variable and function");
if ((t == 0) && tu)
}
// check for conflicting variable declaration with function
// do not use diag() because line number might be misleading
if (is_var_declared_as_function(s)) {
Fprintf(stderr, "Error: %s used as both variable and function in file %s\n", s->name, finname);
exit(1);
}
if ((t == 0) && tu) {
Fprintf(stderr, "Warning: %s undefined. (declared within VERBATIM?)\n", s->name);
}
}
if (err) {
diag("multiple uses for same variable", (char*) 0);
Expand Down
2 changes: 1 addition & 1 deletion src/nmodl/io.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ char* current_line() { /* assumes we actually want the previous line */
/* two arguments so we can pass a name to construct an error message. */
void diag(char* s1, char* s2) {
char* cp;
Fprintf(stderr, "%s", s1);
Fprintf(stderr, "Error: %s", s1);
if (s2) {
Fprintf(stderr, "%s", s2);
}
Expand Down
18 changes: 18 additions & 0 deletions src/nmodl/modl.h
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,24 @@ typedef struct Symbol {
#define EXTDEF5 040000000L /* not threadsafe from the extdef list */
#define EXPLICIT_DECL 01 /* usage field, variable occurs in input file */


#define NRNEXTRN 01 /* t, dt, celsius, etc. */
#define NRNCURIN 02 /* input value used */
#define NRNCUROUT 04 /* added to output value */
#define NRNRANGE 010
#define NRNPRANGEIN 020
#define NRNPRANGEOUT 040
#define NRNGLOBAL 0100 /* same for all sections, defined here */
#define NRNSTATIC 0200 /* v */
#define NRNNOTP 0400 /* doesn't belong in p array */
#define NRNIONFLAG 01000 /* temporary flag to allow READ and WRITE \
without declaring twice */
#define NRNSECTION 02000
#define NRNPOINTER 04000
#define IONCONC 010000
#define NRNBBCOREPOINTER 020000


extern char *emalloc(unsigned), /* malloc with out of space checking */
*stralloc(char*, char*), /* copies string to new space */
*inputline(), /* used only by parser to get title line */
Expand Down
16 changes: 0 additions & 16 deletions src/nmodl/nocpout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,22 +85,6 @@ not thread safe and _p and _ppvar are static.
*/

#endif
#define NRNEXTRN 01 /* t, dt, celsius, etc. */
#define NRNCURIN 02 /* input value used */
#define NRNCUROUT 04 /* added to output value */
#define NRNRANGE 010
#define NRNPRANGEIN 020
#define NRNPRANGEOUT 040
#define NRNGLOBAL 0100 /* same for all sections, defined here */
#define NRNSTATIC 0200 /* v */
#define NRNNOTP 0400 /* doesn't belong in p array */
#define NRNIONFLAG \
01000 /* temporary flag to allow READ and WRITE \
without declaring twice */
#define NRNSECTION 02000
#define NRNPOINTER 04000
#define IONCONC 010000
#define NRNBBCOREPOINTER 020000

#define IONEREV 0 /* Parameter */
#define IONIN 1
Expand Down

0 comments on commit 7c4660b

Please sign in to comment.