-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
doc: document nullptr comparisons in style guide #23805
Conversation
This documents existing practices.
I thought we agreed to adopt the C++ Core Guidelines exactly to close the omissions in the style guide, and have explicit and well reasoned guidelines. Could you provide reasons for this change? |
What is says in the PR description. It documents what we currently have. Preferring to document existing style before working on changes was also what was echoed as a sentiment in the last TSC meeting.
We agreed to link these documents since they provide meaningful guidelines. They do not match our current style in all cases; for those cases, we have this very document. |
|
||
Use explicit comparisons to `nullptr` when testing pointers, i.e. | ||
`if (foo == nullptr)` instead of `if (foo)` and | ||
`foo != nullptr` instead of `!foo`. |
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.
For consistency, maybe make this if (foo != nullptr)
and if (!foo)
.
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.
@cjihrig yup, done!
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.
I would like to ask for a reference for where this guideline came from, and what is it's reasoning.
@refack This is, as the PR says, an existing practice that we follow. I can only speculate about the reasons why it was introduced, but it does follow the general “explicit is better than implicit” rule, and makes clear whether a check tests a pointer or a boolean. The origins of the rule definitely predate my time around here (and I’m not so sure about their relevance). @bnoordhuis might know. |
Landed in 24cb1f3. |
This documents existing practices. PR-URL: #23805 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
This documents existing practices. PR-URL: #23805 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
This documents existing practices. PR-URL: #23805 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
This documents existing practices. PR-URL: #23805 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
This documents existing practices. PR-URL: #23805 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
This documents existing practices. PR-URL: #23805 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
This documents existing practices.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes