From 7c4660bc5911dfa38cff1372ea2229db04274a3f Mon Sep 17 00:00:00 2001 From: Pramod S Kumbhar Date: Fri, 23 Sep 2022 13:42:01 +0200 Subject: [PATCH] Prevent user from declaring variables and functions with same name * 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 ``` --- src/nmodl/consist.cpp | 36 ++++++++++++++++++++++++++++++++++-- src/nmodl/io.cpp | 2 +- src/nmodl/modl.h | 18 ++++++++++++++++++ src/nmodl/nocpout.cpp | 16 ---------------- 4 files changed, 53 insertions(+), 19 deletions(-) diff --git a/src/nmodl/consist.cpp b/src/nmodl/consist.cpp index f3958f6449..07dc8c8794 100644 --- a/src/nmodl/consist.cpp +++ b/src/nmodl/consist.cpp @@ -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; @@ -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); diff --git a/src/nmodl/io.cpp b/src/nmodl/io.cpp index f2796b5a70..d1b3fb7cdc 100644 --- a/src/nmodl/io.cpp +++ b/src/nmodl/io.cpp @@ -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); } diff --git a/src/nmodl/modl.h b/src/nmodl/modl.h index fc285e95b7..e9cf9e66b5 100644 --- a/src/nmodl/modl.h +++ b/src/nmodl/modl.h @@ -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 */ diff --git a/src/nmodl/nocpout.cpp b/src/nmodl/nocpout.cpp index 2115dcf556..831e8caa2b 100644 --- a/src/nmodl/nocpout.cpp +++ b/src/nmodl/nocpout.cpp @@ -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