-
Notifications
You must be signed in to change notification settings - Fork 123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
mech_api.h: expose more declarations to translated MOD files #1825
Conversation
b7c8433
to
d0fb3b3
Compare
also need
|
|
Also need
|
Also
is needed |
I think we want Lines 234 to 238 in 571ab16
ctemplate in 8.2.0. Then we can use #ifndef NRN_VERSION_GTEQ_8_2_0 instead of #ifdef __cplusplus .
|
With the current state of this PR there is a problem with To fix this, we should move |
I think the problem is not the type but the (unhelpful) parameter name. In the definition it's called |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1825 +/- ##
=======================================
Coverage 47.05% 47.05%
=======================================
Files 543 543
Lines 112949 112949
=======================================
Hits 53143 53143
Misses 59806 59806 ☔ View full report in Codecov by Sentry. |
Other possible additions:
|
- mcran4.h in mech_api.h - hoc_get_symbol declared - Object member always called ctemplate in 8.2+ - ivoc_list_item has C linkage in C
- Make nrncvode.h dual C/C++ and include it in mech_api.h. - clear_event_queue and cvode_fadvance are used in ModelDB models.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@nrnhines: we are ready to merge this and get 8.2a
tagged today. Before we do this, do you have anything that you would like to have in 8.2a?
As formatting is a nice feature, it would be nice to merge #1820 |
* Fixed intf_6.mod and misc.h for NEURON 9.0.0, 8.2.0, 8.1.0 * Fixed misc.h, misc.mod, stats.mod and vecst.mod for NEURON 9.0.0, 8.2.0 and 8.1.0 - Small issue with hoc_get_symbol with NEURON 8.2.0 that should be fixed by neuronsimulator/nrn#1825 * Updated readme * Fix inclusion of float.h * fixups for new 8.2 wheels * Cleanup for merge. * Object member is called template in C in 8.2 * include with "..." not <...> * mcell_ran4_init: pass a value * mcell_ran4_init: update readme * hashseed2: probable bugfix * s-0x0 -> s=0x0: probably typo (and compiler warning) fix Co-authored-by: Olli Lupton <[email protected]> Co-authored-by: Pramod S Kumbhar <[email protected]>
Include more headers in
mech_api.h
. This means that when updating MOD files to be compatible with C++ / #1597 then we can assume that NEURON-internal methods do not need to be declared for versions >=8.2.As a brief reminder of the plan:
master
after this PR is mergedmaster
after Compile NEURON mechanisms as C++ #1597 is mergedIn this PR:
mcell_ran4_init
,mcell_ran4
, ... are declaredmcell_ran4_init
exist in ModelDB (example: https://github.com/ModelDBRepository/106891/blob/7f825c80ef3a76fb181607a346de730b8f740005/stats.mod#L56), and exposing the correct declaration breaks compilation.ivoc_list_item
is declaredhoc_get_symbol
is declaredcvode_fadvance
,clear_event_queue
, ... are declared(description by @olupton, don't blame @alexsavulescu for it)