nrn icon indicating copy to clipboard operation
nrn copied to clipboard

NEURON's preprocessor usage makes it difficult to use the C++ standard library

Open olupton opened this issue 1 year ago • 1 comments

Context

In some contexts NEURON includes https://github.com/neuronsimulator/nrn/blob/056cceaf8a6e447550cf72348d7826f8f5991735/src/nrnoc/md1redef.h and https://github.com/neuronsimulator/nrn/blob/056cceaf8a6e447550cf72348d7826f8f5991735/src/nrnoc/md2redef.h before/after some other NEURON headers: https://github.com/neuronsimulator/nrn/blob/056cceaf8a6e447550cf72348d7826f8f5991735/src/nmodl/noccout.cpp#L136-L139

Overview of the issue

These include things like https://github.com/neuronsimulator/nrn/blob/056cceaf8a6e447550cf72348d7826f8f5991735/src/nrnoc/md1redef.h#L25 and https://github.com/neuronsimulator/nrn/blob/056cceaf8a6e447550cf72348d7826f8f5991735/src/nrnoc/md2redef.h#L17 which can cause various problems if the headers included in between do things like calling std::vector::data().

Presumably these macros are define to try and avoid clashes between variable names in NEURON data structures and NMODL variable names in translated MOD files, which nocmodl defines macros for.

This came up (again) in the context of https://github.com/neuronsimulator/nrn/pull/1929.

Expected result/behaviour

It should be possible to use standard library components in headers like section.h.

Probably the simplest solution is to rename the relevant variables consistently in the NEURON codebase, using one of the underscore pre/postfix patterns that is already widely used, and then to drop these redefinition headers.

Note that some names including underscore prefixes are reserved in C++ (https://en.cppreference.com/w/cpp/language/identifiers):

  • the identifiers with a double underscore anywhere are reserved;
  • the identifiers that begin with an underscore followed by an uppercase letter are reserved;
  • the identifiers that begin with an underscore are reserved in the global namespace.

Does this sound reasonable @nrnhines ? cc: @alkino

olupton avatar Jul 26 '22 09:07 olupton

I'm ok with name changes to avoid define undef etc. If a nrn_ prefix will do the job, I'm partial to that.

nrnhines avatar Jul 26 '22 15:07 nrnhines