neuronsimulator / nrn

NEURON Simulator
http://nrn.readthedocs.io
Other
393 stars 115 forks source link

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

Open olupton opened 2 years ago

olupton commented 2 years ago

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):

Does this sound reasonable @nrnhines ? cc: @alkino

nrnhines commented 2 years ago

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