Don’t use zero as null pointer constant. Use nullptr (C++11). #13802

1. 
   practicalswift commented at 10:44 am on July 30, 2018: contributor

   Don’t use zero as null pointer constant.

   From the developer notes:

   nullptr is preferred over NULL or (void*)0.

   From the C++ Core Guidelines:

   ES.47: Use nullptr rather than 0 or NULL Reason Readability. Minimize surprises: nullptr cannot be confused with an int. nullptr also has a well-specified (very restrictive) type, and thus works in more scenarios where type deduction might do the wrong thing on NULL or 0.

   Found by compiling the project with -Wzero-as-null-pointer-constant and fixing where appropriate (skipping false positives).

2. fanquake added the label Refactoring on Jul 30, 2018
3. practicalswift force-pushed on Jul 30, 2018
4. 
   promag commented at 11:05 am on July 30, 2018: member

   Concept ACK.
5. 
   DrahtBot commented at 11:59 am on July 30, 2018: member

   • #13216 ([Qt] implements concept for different disk sizes on intro by marcoagner)
   • #12288 ([WIP][NET] Add NATPMP support. by annanay25)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

6. 
   fanquake commented at 1:03 pm on July 30, 2018: member

   If this is going to be done, shouldn’t -Wzero-as-null-pointer-constant or a similar sort of lint script be added to the build system to try and stop it’s usage re-occurring? Otherwise this seems like it could end up similar to -Wshadow.
7. 
   practicalswift commented at 1:11 pm on July 30, 2018: contributor

   @fanquake Yes that would be the best way to make sure we don’t regress! Unfortunately -Wzero-as-null-pointer-constant generates some false positives in our code and some true positives in our dependencies.
8. 
   MarcoFalke commented at 1:12 pm on July 30, 2018: member

   Unfortunately -Wzero-as-null-pointer-constant generates some false positives in our code and some true positives in our dependencies.

   In which case I suggest to do the same as with -Wshadow: Don’t enforce it and don’t change existing code. So I tend to Concept NACK here.

9. 
   practicalswift commented at 1:32 pm on July 30, 2018: contributor

   @MarcoFalke Oh, I’d love enabling that warning but in contrast to shadowing I think the probably of regressions is much lower – hopefully making this a one time “switch to C++11” change. New code is unlikely to use 0 instead of the proper pointer literal nullptr, no?

   I’d assume the instances changed here are from the pre-C++11 era.

10. practicalswift renamed this:
    Don't use zero as null pointer constant
    Don't use zero as null pointer constant. Use nullptr (C++11).
    on Jul 30, 2018
11. 
    MarcoFalke commented at 3:57 pm on July 30, 2018: member

    Fine, nullptr is type safe, so at least it is an improvement and also doesn’t hurt. There shouldn’t be any issues given that this compiles, right?
12. 
    practicalswift commented at 4:22 pm on July 30, 2018: contributor

    @MarcoFalke Correct! Type safety is our friend.

    This change is safe:

    • Changing from 0 (or NULL) to nullptr where the zero is used as a null pointer constant is always safe.
    • Changing from 0 (or NULL) to nullptr where the zero is NOT used as a null pointer constant will result in a compiler error.

    So yes, if it compiles it is safe :-)

13. Don't use zero as null pointer constant (-Wzero-as-null-pointer-constant) 08c27602d3
14. practicalswift force-pushed on Jul 30, 2018
15. 
    kallewoof commented at 6:39 pm on July 30, 2018: member

    Weak concept NAK, but I’m okay with it if others are (I understand it’s safe if it compiles).

    Too many files changed (100 lines over 58 files) for a change that isn’t really a fix (the reason stated in C++ guidelines is ‘readability’, which is rather weak for a 58 file change IMO). I feel this is one of those things that can be fixed gradually over time as people update the transgressing code.

16. 
    practicalswift commented at 7:55 pm on July 30, 2018: contributor

    Closing this PR due to lack of consensus ACK :-)
17. practicalswift closed this on Jul 30, 2018

    ---

18. practicalswift deleted the branch on Apr 10, 2021
19. DrahtBot locked this on Aug 16, 2022

Contributors
practicalswift promag DrahtBot fanquake MarcoFalke kallewoof

Labels
Refactoring