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.

Sign up for GitHub

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

Jump to bottom

Modernize the codebase - convert C-style casts to static_casts #2506

Merged
merged 6 commits into from
Aug 7, 2023
Merged

Modernize the codebase - convert C-style casts to static_casts #2506

merged 6 commits into from
Aug 7, 2023

Conversation

JanVogelsang
Copy link
Contributor

This PR converts all old C-style casts of the form (type) value to modern C++ casts of the form static_cast<type>(value).

@JanVogelsang
Copy link
Contributor Author

The PR is based on #2498, which should therefore be merged first.

@JanVogelsang JanVogelsang mentioned this pull request Oct 17, 2022
Merged
@jougs jougs changed the title Modernizing the codebase - Convert C-style casts to static_casts Modernize the codebase - convert C-style casts to static_casts Oct 19, 2022
@jougs jougs added S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation. I: Internal API Changes were introduced in basic internal workings of the simulator that developers need to know labels Oct 19, 2022
@JanVogelsang JanVogelsang marked this pull request as ready for review October 20, 2022 11:26
@heplesser
Copy link
Contributor

@JanVogelsang I am surprised that there are many changes in this one that I thought we had handled in previous PRs (boolean operators especially). Were they overlooked? Could you also pull master and ensure that pp_pop_psc_delta and iaf_psc_alpha_canon are not re-introduced?

@JanVogelsang
Copy link
Contributor Author

As this PR was based on #2498, all these boolean operator changes will be gone after merging master into this branch.

@JanVogelsang
Copy link
Contributor Author

Why does git not remove files on a branch when merging with a branch in which they were removed? I know, because they changed in the branch that gets merged with the other one, but shouldn't that at least result in a merge conflict then instead of just silently re-introducing these files again?

heplesser
heplesser approved these changes Nov 30, 2022
View reviewed changes
Copy link
Contributor

@heplesser heplesser left a comment

Choose a reason for hiding this comment

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

@JanVogelsang Thanks, looks good! Just one suggestion for a follow-up issue.

nestkernel/connection_creator_impl.h Outdated Show resolved Hide resolved
@terhorstd terhorstd added this to PRs in progress in Kernel via automation Jan 17, 2023
@github-actions
Copy link

Pull request automatically marked stale!

@github-actions github-actions bot added the stale Automatic marker for inactivity, please have another look here label Jan 30, 2023
@jougs
Copy link
Contributor

jougs commented Feb 1, 2023

@JanVogelsang: Could you please merge master again? It somehow feels like the diff list is not completely up-to-date. Thanks!

@JanVogelsang
Copy link
Contributor Author

Done!

@github-actions github-actions bot removed the stale Automatic marker for inactivity, please have another look here label Feb 2, 2023
jougs
jougs reviewed Feb 6, 2023
View reviewed changes
Copy link
Contributor

@jougs jougs left a comment

Choose a reason for hiding this comment

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

I think technically we can merge this as is. However, I hear almost each of the changes begging for refactoring, so the casts are not needed in the first place. I will thus not yet approve.

models/ac_generator.cpp Outdated Show resolved Hide resolved
@jougs
Copy link
Contributor

jougs commented Feb 6, 2023

See also #2541.

@JanVogelsang JanVogelsang marked this pull request as draft February 10, 2023 10:35
@JanVogelsang JanVogelsang mentioned this pull request Mar 9, 2023
Merged
@github-actions
Copy link

Pull request automatically marked stale!

@github-actions github-actions bot added the stale Automatic marker for inactivity, please have another look here label Apr 23, 2023
@jougs
Copy link
Contributor

jougs commented Jun 22, 2023

@JanVogelsang: Please merge master and tell me to re-review when you are done. Thanks!

@github-actions github-actions bot removed the stale Automatic marker for inactivity, please have another look here label Jun 23, 2023
JanVogelsang and others added 2 commits July 20, 2023 15:46
@JanVogelsang
Merge branch 'master' into refactoring-casts
2818257 
# Conflicts:
#	libnestutil/stopwatch.h
#	models/ac_generator.cpp
#	models/aeif_cond_alpha.cpp
#	models/aeif_cond_alpha_multisynapse.cpp
#	models/aeif_cond_beta_multisynapse.cpp
#	models/aeif_cond_exp.cpp
#	models/aeif_psc_alpha.cpp
#	models/aeif_psc_delta.cpp
#	models/aeif_psc_delta_clopath.cpp
#	models/aeif_psc_exp.cpp
#	models/amat2_psc_exp.cpp
#	models/binary_neuron.h
#	models/cm_default.cpp
#	models/dc_generator.cpp
#	models/gamma_sup_generator.cpp
#	models/gif_cond_exp.cpp
#	models/gif_cond_exp_multisynapse.cpp
#	models/gif_pop_psc_exp.cpp
#	models/gif_psc_exp.cpp
#	models/gif_psc_exp_multisynapse.cpp
#	models/hh_cond_beta_gap_traub.cpp
#	models/hh_cond_exp_traub.cpp
#	models/hh_psc_alpha.cpp
#	models/hh_psc_alpha_clopath.cpp
#	models/hh_psc_alpha_gap.cpp
#	models/ht_neuron.cpp
#	models/iaf_chs_2007.cpp
#	models/iaf_chxk_2008.cpp
#	models/iaf_cond_alpha.cpp
#	models/iaf_cond_alpha_mc.cpp
#	models/iaf_cond_beta.cpp
#	models/iaf_cond_exp.cpp
#	models/iaf_cond_exp_sfa_rr.cpp
#	models/iaf_psc_alpha.cpp
#	models/iaf_psc_alpha_multisynapse.cpp
#	models/iaf_psc_delta.cpp
#	models/iaf_psc_exp.cpp
#	models/iaf_psc_exp_htum.cpp
#	models/iaf_psc_exp_multisynapse.cpp
#	models/inhomogeneous_poisson_generator.cpp
#	models/izhikevich.cpp
#	models/mat2_psc_exp.cpp
#	models/noise_generator.cpp
#	models/parrot_neuron.cpp
#	models/poisson_generator.cpp
#	models/poisson_generator_ps.cpp
#	models/pp_cond_exp_mc_urbanczik.cpp
#	models/pp_psc_delta.cpp
#	models/ppd_sup_generator.cpp
#	models/rate_neuron_ipn_impl.h
#	models/rate_neuron_opn_impl.h
#	models/rate_transformer_node_impl.h
#	models/siegert_neuron.cpp
#	models/sinusoidal_gamma_generator.cpp
#	models/sinusoidal_poisson_generator.cpp
#	models/step_current_generator.cpp
#	models/step_rate_generator.cpp
#	nestkernel/connection_creator_impl.h
#	nestkernel/event_delivery_manager.cpp
#	nestkernel/ring_buffer.h
#	nestkernel/simulation_manager.cpp
#	nestkernel/slice_ring_buffer.h
@JanVogelsang
Removed unnecessary casts
721837a
@JanVogelsang JanVogelsang marked this pull request as ready for review July 20, 2023 13:58
@JanVogelsang
Copy link
Contributor Author

@jougs You can now re-review. Most of the casts are actually gone now due to #2616 and #2541. I removed all unnecessary casts that I could find.

@jougs jougs merged commit d31b9a2 into nest:master Aug 7, 2023
20 checks passed
Kernel automation moved this from PRs in progress to Done Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@heplesser heplesser heplesser approved these changes

@jougs jougs jougs approved these changes

Assignees
No one assigned
Labels
I: Internal API Changes were introduced in basic internal workings of the simulator that developers need to know S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation.
Projects
Kernel
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@JanVogelsang @heplesser @jougs