build: Optionally enable -Wzero-as-null-pointer-constant #15112

Empact · 2019-01-05T22:47:42Z

Note: This is based on #14920

This enables -Wzero-as-null-pointer-constant, while avoiding applying it to dependencies.

~~Used Qt::NoItemFlags, Qt::Widget instead where applicable.~~

See #10483, #10645, #13802 for more on nullptr.

practicalswift · 2019-01-05T23:07:52Z

Concept ACK and in accordance with our developer notes.

Thanks for doing this!

Using zero as null pointer constant is sloppy at best and dangerous at worst :-)

hebasto · 2019-01-06T09:21:28Z

Concept ACK.

maflcko · 2019-01-06T09:34:03Z

Could submit the src/qt changes as separate pull request, since those are the bulk of the changes?

practicalswift · 2019-01-06T09:38:15Z

@MarcoFalke Perhaps 10f81e916d5fbb2751d03d06901f68d9b507807c (including the two non QT fixes) can be posted as a separate PR since they are all trivial to review?

practicalswift · 2019-01-07T15:30:25Z

@Empact What about -Werror=zero-as-null-pointer-constant as discussed in #15114 (comment)?

Empact · 2019-01-07T19:04:57Z

Enabled now. 👍

practicalswift · 2019-01-07T21:07:07Z

utACK 873145a56315ba032aa100646d682ec31d39e8d9

Empact · 2019-01-09T06:23:11Z

As per conversation with @theuni, this now is only active if the user configures with --enable-isystem, so the build possibilities are:
default: disabled
--enable-isystem: -Wzero-as-null-pointer-constant
--enable-isystem --enable-werror: -Werror=zero-as-null-pointer-constant

The idea being to add a travis run that exercises the latter combination to deny the introduction of violations.
#14920 (comment)

DrahtBot · 2019-01-09T22:49:59Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#24914 (wallet: Load database records in a particular order by achow101)
#24322 ([kernel 1/n] Introduce initial libbitcoinkernel by dongcarl)
#24185 (refactor: only use explicit reinterpret/const casts, not implicit by PastaPastaPasta)

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.

Empact · 2019-01-13T17:48:15Z

Rebased for #13216

Empact · 2019-01-14T08:43:37Z

Rebased for #15154

@MarcoFalke

3a0e76f Replace remaining 0 with nullptr in Qt code (Ben Woosley) 9096276 Don't use zero as null pointer constant (-Wzero-as-null-pointer-constant) (practicalswift) Pull request description: This corrects all violations of `-Wzero-as-null-pointer-constant` identified in the Qt codebase. These changes are extracted from bitcoin#15112 as suggested by @MarcoFalke to ease review. This is in service of enabling `-Wzero-as-null-pointer-constant`, which should eliminate this as a concern going forward. Note there are 2 non-Qt changes: `src/test/allocator_tests.cpp` and `src/wallet/db.cpp`. Tree-SHA512: 206bd668802147ba42bc413c2d7d259cb59aca9ec1da74a6bf2ca3932e60ae492faacbc61bcee0fd6b4b49a4d59d075b7e5404f0526b36c47718f9b0587e7768

…ant)

…_warnings And -Werror=zero-as-null-pointer-constant if werror are enabled. While disabling it in the following dependencies: * LevelDb via build flags * Tinyformat via pragmas due to external library * Sqlite via pragmas due to use of SQLITE_STATIC * util_tests via pragmas due to use of SIG_DFL

As without this, GCC warns on the following unrecognized clang pragmas. bitcoin#15112 (comment)

Empact · 2022-04-22T21:25:11Z

Rebase for the move of SQLITE_STATIC that occurred in #23732

As without this, GCC warns on the following unrecognized clang pragmas. bitcoin#15112 (comment)

fanquake · 2022-04-26T10:15:34Z

Going to close this in favour of #24971; where we outsource this check to external tooling, and don't have to cover the codebase in pragmas to satisfy compilers.

9c96f10 tidy: enable modernize-use-nullptr (fanquake) e532748 Don't use zero as null pointer constant (-Wzero-as-null-pointer-constant) (practicalswift) Pull request description: Alternative to #15112 which uses `clang-tidy` to do perform the checking, rather than `-Wzero-as-null-pointer-constant`, and avoids having to uses pragmas, i.e: ```cpp #if defined(HAVE_CONFIG_H) #include <config/bitcoin-config.h> #endif #if defined(HAVE_W_ZERO_AS_NULL_POINTER_CONSTANT) #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wzero-as-null-pointer-constant" #pragma GCC diagnostic ignored "-Wunknown-pragmas" #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wzero-as-null-pointer-constant" #endif ``` to suppress warnings coming from upstream code. Can be tested by dropping the preceding commit. Should produce errors like: ```bash clang-tidy-14 --use-color -p=/home/ubuntu/ci_scratch/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu /home/ubuntu/ci_scratch/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/netbase.cpp /home/ubuntu/ci_scratch/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/netbase.cpp:678:36: error: use nullptr [modernize-use-nullptr,-warnings-as-errors] if (!Socks5(strDest, port, 0, sock)) { ^ nullptr ``` ACKs for top commit: laanwj: Concept and code review ACK 9c96f10 Tree-SHA512: d822a354e44ba8f7fc53da9a4be7de5c25cc4ffc7c57651b76fdd1a030764b0390cfd79fca94685b8a3ff4f4d13054764f12d1f0d8c2a1b9ba519a7524f7f5bf

Empact · 2022-04-26T17:57:44Z

Sounds good, one of these days I'll familiarize myself with clang-tidy.

9c96f10 tidy: enable modernize-use-nullptr (fanquake) e532748 Don't use zero as null pointer constant (-Wzero-as-null-pointer-constant) (practicalswift) Pull request description: Alternative to bitcoin#15112 which uses `clang-tidy` to do perform the checking, rather than `-Wzero-as-null-pointer-constant`, and avoids having to uses pragmas, i.e: ```cpp #if defined(HAVE_CONFIG_H) #include <config/bitcoin-config.h> #endif #if defined(HAVE_W_ZERO_AS_NULL_POINTER_CONSTANT) #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wzero-as-null-pointer-constant" #pragma GCC diagnostic ignored "-Wunknown-pragmas" #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wzero-as-null-pointer-constant" #endif ``` to suppress warnings coming from upstream code. Can be tested by dropping the preceding commit. Should produce errors like: ```bash clang-tidy-14 --use-color -p=/home/ubuntu/ci_scratch/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu /home/ubuntu/ci_scratch/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/netbase.cpp /home/ubuntu/ci_scratch/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/netbase.cpp:678:36: error: use nullptr [modernize-use-nullptr,-warnings-as-errors] if (!Socks5(strDest, port, 0, sock)) { ^ nullptr ``` ACKs for top commit: laanwj: Concept and code review ACK 9c96f10 Tree-SHA512: d822a354e44ba8f7fc53da9a4be7de5c25cc4ffc7c57651b76fdd1a030764b0390cfd79fca94685b8a3ff4f4d13054764f12d1f0d8c2a1b9ba519a7524f7f5bf

@MarcoFalke

3a0e76f Replace remaining 0 with nullptr in Qt code (Ben Woosley) 9096276 Don't use zero as null pointer constant (-Wzero-as-null-pointer-constant) (practicalswift) Pull request description: This corrects all violations of `-Wzero-as-null-pointer-constant` identified in the Qt codebase. These changes are extracted from bitcoin#15112 as suggested by @MarcoFalke to ease review. This is in service of enabling `-Wzero-as-null-pointer-constant`, which should eliminate this as a concern going forward. Note there are 2 non-Qt changes: `src/test/allocator_tests.cpp` and `src/wallet/db.cpp`. Tree-SHA512: 206bd668802147ba42bc413c2d7d259cb59aca9ec1da74a6bf2ca3932e60ae492faacbc61bcee0fd6b4b49a4d59d075b7e5404f0526b36c47718f9b0587e7768

9c96f10 tidy: enable modernize-use-nullptr (fanquake) e532748 Don't use zero as null pointer constant (-Wzero-as-null-pointer-constant) (practicalswift) Pull request description: Alternative to bitcoin#15112 which uses `clang-tidy` to do perform the checking, rather than `-Wzero-as-null-pointer-constant`, and avoids having to uses pragmas, i.e: ```cpp #if defined(HAVE_CONFIG_H) #include <config/bitcoin-config.h> #endif #if defined(HAVE_W_ZERO_AS_NULL_POINTER_CONSTANT) #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wzero-as-null-pointer-constant" #pragma GCC diagnostic ignored "-Wunknown-pragmas" #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wzero-as-null-pointer-constant" #endif ``` to suppress warnings coming from upstream code. Can be tested by dropping the preceding commit. Should produce errors like: ```bash clang-tidy-14 --use-color -p=/home/ubuntu/ci_scratch/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu /home/ubuntu/ci_scratch/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/netbase.cpp /home/ubuntu/ci_scratch/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/netbase.cpp:678:36: error: use nullptr [modernize-use-nullptr,-warnings-as-errors] if (!Socks5(strDest, port, 0, sock)) { ^ nullptr ``` ACKs for top commit: laanwj: Concept and code review ACK 9c96f10 Tree-SHA512: d822a354e44ba8f7fc53da9a4be7de5c25cc4ffc7c57651b76fdd1a030764b0390cfd79fca94685b8a3ff4f4d13054764f12d1f0d8c2a1b9ba519a7524f7f5bf

Empact force-pushed the zero-as-null-pointer-constant branch from 01a7e89 to 3bca8e9 Compare January 5, 2019 22:48

fanquake added the Build system label Jan 5, 2019

Empact mentioned this pull request Jan 6, 2019

Build: enable -Wdocumentation via isystem #14920

Closed

Empact force-pushed the zero-as-null-pointer-constant branch 2 times, most recently from 9724c7c to 6d509e7 Compare January 6, 2019 03:38

Empact mentioned this pull request Jan 6, 2019

Qt: Replace remaining 0 with nullptr #15114

Merged

Empact force-pushed the zero-as-null-pointer-constant branch from 6d509e7 to 873145a Compare January 7, 2019 19:04

Empact changed the title ~~build: Enable -Wzero-as-null-pointer-constant~~ build: Enable -Werror=zero-as-null-pointer-constant Jan 7, 2019

Empact force-pushed the zero-as-null-pointer-constant branch from 873145a to 05ad601 Compare January 9, 2019 06:20

Empact changed the title ~~build: Enable -Werror=zero-as-null-pointer-constant~~ build: Optionally enable -Wzero-as-null-pointer-constant Jan 9, 2019

Empact force-pushed the zero-as-null-pointer-constant branch from 05ad601 to 706ac51 Compare January 9, 2019 06:31

DrahtBot added the Needs rebase label Jan 12, 2019

Empact force-pushed the zero-as-null-pointer-constant branch from 706ac51 to 08d8902 Compare January 13, 2019 17:16

DrahtBot removed the Needs rebase label Jan 13, 2019

DrahtBot added the Needs rebase label Jan 13, 2019

Empact force-pushed the zero-as-null-pointer-constant branch from 08d8902 to 48d05f3 Compare January 14, 2019 08:43

DrahtBot removed the Needs rebase label Jan 14, 2019

Empact force-pushed the zero-as-null-pointer-constant branch from 48d05f3 to 5bd3177 Compare January 14, 2019 23:50

DrahtBot added the Needs rebase label Oct 13, 2021

Empact force-pushed the zero-as-null-pointer-constant branch from 7f0f2ca to fcd0a99 Compare October 14, 2021 13:18

DrahtBot removed the Needs rebase label Oct 14, 2021

DrahtBot mentioned this pull request Nov 10, 2021

build: boring autotools cleanup #23473

Merged

DrahtBot added the Needs rebase label Nov 13, 2021

Empact force-pushed the zero-as-null-pointer-constant branch from fcd0a99 to 1b06e99 Compare February 28, 2022 16:15

DrahtBot removed the Needs rebase label Feb 28, 2022

Empact force-pushed the zero-as-null-pointer-constant branch 2 times, most recently from f036c51 to ff29b16 Compare March 2, 2022 16:55

DrahtBot mentioned this pull request Mar 18, 2022

[kernel 1/n] Introduce initial libbitcoinkernel #24322

Merged

DrahtBot mentioned this pull request Apr 8, 2022

refactor: only use explicit reinterpret/const casts, not implicit #24185

Closed

DrahtBot mentioned this pull request Apr 19, 2022

wallet: Load database records in a particular order #24914

Merged

practicalswift and others added 2 commits April 22, 2022 16:04

Don't use zero as null pointer constant (-Wzero-as-null-pointer-const…

f1bdd58

…ant)

Empact added a commit to Empact/bitcoin that referenced this pull request Apr 22, 2022

Ignore gcc -Wunknown-pragmas in tinyformat.h

58ca282

As without this, GCC warns on the following unrecognized clang pragmas. bitcoin#15112 (comment)

Empact force-pushed the zero-as-null-pointer-constant branch from ff29b16 to 58ca282 Compare April 22, 2022 21:23

Ignore gcc -Wunknown-pragmas in tinyformat.h

0b3c355

As without this, GCC warns on the following unrecognized clang pragmas. bitcoin#15112 (comment)

Empact force-pushed the zero-as-null-pointer-constant branch from 58ca282 to 0b3c355 Compare April 22, 2022 21:26

fanquake mentioned this pull request Apr 25, 2022

tidy: modernize-use-nullptr #24971

Merged

fanquake closed this Apr 26, 2022

Empact deleted the zero-as-null-pointer-constant branch April 26, 2022 17:57

Empact restored the zero-as-null-pointer-constant branch November 18, 2022 20:47

bitcoin locked and limited conversation to collaborators Nov 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

build: Optionally enable -Wzero-as-null-pointer-constant #15112

build: Optionally enable -Wzero-as-null-pointer-constant #15112

Uh oh!

Empact commented Jan 5, 2019 •

edited

Loading

Uh oh!

practicalswift commented Jan 5, 2019 •

edited

Loading

Uh oh!

hebasto commented Jan 6, 2019

Uh oh!

maflcko commented Jan 6, 2019

Uh oh!

practicalswift commented Jan 6, 2019

Uh oh!

practicalswift commented Jan 7, 2019

Uh oh!

Empact commented Jan 7, 2019

Uh oh!

practicalswift commented Jan 7, 2019

Uh oh!

Empact commented Jan 9, 2019

Uh oh!

DrahtBot commented Jan 9, 2019 •

edited

Loading

Uh oh!

Empact commented Jan 13, 2019

Uh oh!

Empact commented Jan 14, 2019

Uh oh!

Empact commented Apr 22, 2022

Uh oh!

fanquake commented Apr 26, 2022

Uh oh!

Empact commented Apr 26, 2022

Uh oh!

Uh oh!

build: Optionally enable -Wzero-as-null-pointer-constant #15112

build: Optionally enable -Wzero-as-null-pointer-constant #15112

Uh oh!

Conversation

Empact commented Jan 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

practicalswift commented Jan 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Jan 6, 2019

Uh oh!

maflcko commented Jan 6, 2019

Uh oh!

practicalswift commented Jan 6, 2019

Uh oh!

practicalswift commented Jan 7, 2019

Uh oh!

Empact commented Jan 7, 2019

Uh oh!

practicalswift commented Jan 7, 2019

Uh oh!

Empact commented Jan 9, 2019

Uh oh!

DrahtBot commented Jan 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

Empact commented Jan 13, 2019

Uh oh!

Empact commented Jan 14, 2019

Uh oh!

Empact commented Apr 22, 2022

Uh oh!

fanquake commented Apr 26, 2022

Uh oh!

Empact commented Apr 26, 2022

Uh oh!

Uh oh!

Empact commented Jan 5, 2019 •

edited

Loading

practicalswift commented Jan 5, 2019 •

edited

Loading

DrahtBot commented Jan 9, 2019 •

edited

Loading