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

Modernize the codebase - check for nullptr #2496

Merged
merged 6 commits into from
Oct 10, 2022

Conversation

JanVogelsang
Copy link
Contributor

@JanVogelsang JanVogelsang commented Oct 7, 2022

This PR is based on PR #2493.
In the C++ standard it is defined that if ( ptr != nullptr ) gives the same result as if ( ptr ) as ptr is converted to a bool in the latter case, which per standard evaluates to true only if ptr != nullptr.
We can therefore convert all occurrences of
if ( ptr != nullptr ) to if ( ptr )
and
if ( ptr == nullptr ) to if ( not ptr )
which makes it quite clear in which cases a pointer is expected to be valid and when not.

@heplesser heplesser added S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation. I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels Oct 7, 2022
@heplesser heplesser added this to PRs in progress in Kernel via automation Oct 7, 2022
@jougs jougs marked this pull request as ready for review October 10, 2022 08:24
models/aeif_psc_delta.cpp Outdated Show resolved Hide resolved
models/aeif_psc_delta.cpp Outdated Show resolved Hide resolved
models/aeif_psc_delta.cpp Outdated Show resolved Hide resolved
@JanVogelsang
Copy link
Contributor Author

Not sure why, but after the latest commit, I now double-checked both == nullptr and != nullptr and did not find any more occurrences.

@JanVogelsang JanVogelsang marked this pull request as draft October 10, 2022 10:12
@JanVogelsang JanVogelsang marked this pull request as ready for review October 10, 2022 12:01
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.

What can I say?!

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 Looks good, thank you! In the next PR (replacing &&, ||, and ! with and, or and not), it would be good to also remove superfluous parentheses in boolean expression, e.g. of to form (n1) and (n2).

@heplesser
Copy link
Contributor

And merging ...

@heplesser heplesser merged commit ca47422 into nest:master Oct 10, 2022
Kernel automation moved this from PRs in progress to Done (PRs and issues) Oct 10, 2022
@JanVogelsang JanVogelsang deleted the refactoring-pointer-check branch October 10, 2022 21:39
@JanVogelsang
Copy link
Contributor Author

@JanVogelsang Looks good, thank you! In the next PR (replacing &&, ||, and ! with and, or and not), it would be good to also remove superfluous parentheses in boolean expression, e.g. of to form (n1) and (n2).

I could only find very few occurrences (~25) of redundant parentheses, but I think I got them all. PR is incoming as soon as all tests pass.

@jougs jougs changed the title Modernizing the codebase - checking for nullptr Modernize the codebase - check for nullptr Oct 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation.
Projects
Kernel
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants