Skip to content
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

Remove more C++ transitional measures #1903

Merged
merged 13 commits into from
Jul 7, 2022
Merged

Remove more C++ transitional measures #1903

merged 13 commits into from
Jul 7, 2022

Conversation

olupton
Copy link
Collaborator

@olupton olupton commented Jul 6, 2022

Now #1597 is merged, only some quasi-external code like scopmath, sparse13 and mesch is still compiled as C.
This means that most explicit C linkage is no longer needed, and we can use C++ linkage. Similarly __cplusplus can be assumed to be defined in most cases.

When things are being touched anyway, tend to prefer moving declarations to header files and including those headers:

  • This exposed some inconsistencies: oc_save_hoc_oop and oc_restore_hoc_oop were being called with the wrong number of arguments.

There are some exceptions:

  • The MPI wrappers are used with dlopen, dlsym etc...so for the moment it's simpler to stick with C linkage.
  • Some other entry points are used with dlopen and friends, and keep C linkage.
  • Some NEURON variables are used inside quasi-external code and still need to have C linkage.

Some unused files were deleted:

  • src/nrnoc/cprop.cpp
  • src/oc/spinit.cpp

Questions / deferred items:

  • Does nrn-modeldb-ci pass?

@codecov-commenter
Copy link

codecov-commenter commented Jul 6, 2022

Codecov Report

Merging #1903 (622aeb3) into master (f20c3dc) will decrease coverage by 0.03%.
The diff coverage is 89.36%.

@@            Coverage Diff             @@
##           master    #1903      +/-   ##
==========================================
- Coverage   47.17%   47.13%   -0.04%     
==========================================
  Files         543      543              
  Lines      112912   112910       -2     
==========================================
- Hits        53264    53221      -43     
- Misses      59648    59689      +41     
Impacted Files Coverage Δ
src/ivoc/fourier.cpp 97.67% <ø> (ø)
src/ivoc/graph.cpp 10.38% <ø> (ø)
src/ivoc/idraw.cpp 0.31% <ø> (ø)
src/ivoc/ivoc.cpp 49.06% <ø> (ø)
src/ivoc/ivocmain.cpp 74.28% <ø> (ø)
src/ivoc/ivocvect.cpp 81.79% <0.00%> (ø)
src/ivoc/ivocvect.h 95.12% <ø> (ø)
src/ivoc/mymath.h 47.61% <ø> (ø)
src/ivoc/nrnmain.cpp 100.00% <ø> (ø)
src/ivoc/ocobserv.cpp 100.00% <ø> (ø)
... and 68 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@azure-pipelines
Copy link

✔️ 622aeb3 -> Azure artifacts URL

@olupton olupton marked this pull request as ready for review July 6, 2022 12:42
@azure-pipelines
Copy link

✔️ b8e873c -> Azure artifacts URL

@olupton
Copy link
Collaborator Author

olupton commented Jul 6, 2022

✔️ b8e873c -> Azure artifacts URL

Launched https://github.com/neuronsimulator/nrn-modeldb-ci/actions/runs/2623246528 comparing this with neuron-nightly.

Copy link
Member

@alexsavulescu alexsavulescu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@olupton
Copy link
Collaborator Author

olupton commented Jul 6, 2022

✔️ b8e873c -> Azure artifacts URL

Launched https://github.com/neuronsimulator/nrn-modeldb-ci/actions/runs/2623246528 comparing this with neuron-nightly.

Looks alright to me.

@olupton olupton merged commit f2b2b44 into master Jul 7, 2022
@olupton olupton deleted the olupton/c++-cleanup branch July 7, 2022 05:57
olupton added a commit that referenced this pull request Jul 7, 2022
olupton added a commit that referenced this pull request Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants