build: Optionally enable -Wzero-as-null-pointer-constant by Empact · Pull Request #15112 · bitcoin/bitcoin

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

practicalswift · 2021-04-22T06:21:54Z

cr ACK 7f0f2ca49f23cfd9e28892137e13f057a2152a91: patch looks correct

hebasto · 2021-04-29T17:26:16Z

src/tinyformat.h

With gcc 9.3.0:

./tinyformat.h:134: warning: ignoring #pragma clang diagnostic [-Wunknown-pragmas] 134 | #pragma clang diagnostic push | ./tinyformat.h:135: warning: ignoring #pragma clang diagnostic [-Wunknown-pragmas] 135 | #pragma clang diagnostic ignored "-Wzero-as-null-pointer-constant" |

Might be better to do with clang-tidy or so externally instead of maiming the source code?

Might be better to do with clang-tidy

Have done this in #24971.

hebasto · 2021-04-29T17:27:04Z

src/tinyformat.h

With gcc 9.3.0:

./tinyformat.h:1179: warning: ignoring #pragma clang diagnostic [-Wunknown-pragmas] 1179 | #pragma clang diagnostic pop |

I made the changes in 0b3c355 in an attempt to fix this - willing to test?

hebasto · 2021-04-29T17:31:57Z

src/wallet/sqlite.cpp

If I remove changes in this file I won't receive warning with both gcc 9.3.0 and clang 10.0.0.
What cases are guarded here?

Having system sqlite 3.31.1.

Here's the failure I get on removing that:

CXX wallet/libbitcoin_wallet_a-sqlite.o wallet/sqlite.cpp:379:73: error: zero as null pointer constant [-Werror,-Wzero-as-null-pointer-constant] int res = sqlite3_bind_blob(m_read_stmt, 1, key.data(), key.size(), SQLITE_STATIC); ^ /usr/local/Cellar/sqlite/3.36.0/include/sqlite3.h:5666:54: note: expanded from macro 'SQLITE_STATIC' #define SQLITE_STATIC ((sqlite3_destructor_type)0) ^ wallet/sqlite.cpp:420:66: error: zero as null pointer constant [-Werror,-Wzero-as-null-pointer-constant] int res = sqlite3_bind_blob(stmt, 1, key.data(), key.size(), SQLITE_STATIC); ^ /usr/local/Cellar/sqlite/3.36.0/include/sqlite3.h:5666:54: note: expanded from macro 'SQLITE_STATIC' #define SQLITE_STATIC ((sqlite3_destructor_type)0) ^ wallet/sqlite.cpp:427:66: error: zero as null pointer constant [-Werror,-Wzero-as-null-pointer-constant] res = sqlite3_bind_blob(stmt, 2, value.data(), value.size(), SQLITE_STATIC); ^ /usr/local/Cellar/sqlite/3.36.0/include/sqlite3.h:5666:54: note: expanded from macro 'SQLITE_STATIC' #define SQLITE_STATIC ((sqlite3_destructor_type)0) ^ wallet/sqlite.cpp:451:75: error: zero as null pointer constant [-Werror,-Wzero-as-null-pointer-constant] int res = sqlite3_bind_blob(m_delete_stmt, 1, key.data(), key.size(), SQLITE_STATIC); ^ /usr/local/Cellar/sqlite/3.36.0/include/sqlite3.h:5666:54: note: expanded from macro 'SQLITE_STATIC' #define SQLITE_STATIC ((sqlite3_destructor_type)0) ^ wallet/sqlite.cpp:476:73: error: zero as null pointer constant [-Werror,-Wzero-as-null-pointer-constant] int res = sqlite3_bind_blob(m_read_stmt, 1, key.data(), key.size(), SQLITE_STATIC); ^ /usr/local/Cellar/sqlite/3.36.0/include/sqlite3.h:5666:54: note: expanded from macro 'SQLITE_STATIC' #define SQLITE_STATIC ((sqlite3_destructor_type)0) ^ 5 errors generated. make[2]: *** [wallet/libbitcoin_wallet_a-sqlite.o] Error 1

BTW:

% clang --version Apple clang version 12.0.5 (clang-1205.0.22.11) Target: x86_64-apple-darwin20.6.0 Thread model: posix InstalledDir: /Library/Developer/CommandLineTools/usr/bin

@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

@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

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.

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

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

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 mentioned this pull request Apr 13, 2021

build: Add -Werror=missing-noreturn #21667

Closed

Empact force-pushed the zero-as-null-pointer-constant branch from 0d37ca7 to 7313fd0 Compare April 14, 2021 05:01

Empact force-pushed the zero-as-null-pointer-constant branch from 7313fd0 to 7f0f2ca Compare April 21, 2021 20:49

hebasto reviewed Apr 29, 2021

View reviewed changes

DrahtBot mentioned this pull request May 24, 2021

build: Make --enable-suppress-external-warnings the default #22041

Closed

DrahtBot mentioned this pull request Oct 1, 2021

build: make --enable-werror just -Werror #23149

Merged

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 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)

Ignore gcc -Wunknown-pragmas in tinyformat.h

0b3c355

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

fanquake mentioned this pull request Apr 25, 2022

tidy: modernize-use-nullptr #24971

Merged

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!

practicalswift commented Apr 22, 2021

Uh oh!

hebasto Apr 29, 2021

Choose a reason for hiding this comment

Uh oh!

maflcko Apr 23, 2022

Choose a reason for hiding this comment

Uh oh!

fanquake Apr 25, 2022

Choose a reason for hiding this comment

Uh oh!

hebasto Apr 29, 2021

Choose a reason for hiding this comment

Uh oh!

Empact Apr 22, 2022

Choose a reason for hiding this comment

Uh oh!

hebasto Apr 29, 2021

Choose a reason for hiding this comment

Uh oh!

Empact Oct 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Empact commented Apr 22, 2022

Uh oh!

fanquake commented Apr 26, 2022

Uh oh!

Empact commented Apr 26, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

Empact commented Jan 5, 2019 •

edited

Loading

practicalswift commented Jan 5, 2019 •

edited

Loading

DrahtBot commented Jan 9, 2019 •

edited

Loading

Empact Oct 14, 2021 •

edited

Loading