Page MenuHomePhabricator

introduce CharNotInt8 and BasicByte C++20 concepts
Changes PlannedPublic

Authored by PiRK on Wed, Dec 3, 15:56.

Details

Reviewers
None
Group Reviewers
Restricted Project
Summary

This allows removing some Bitcoin ABC specific code and is a dependency for future backports.

We also get better compiler error messages and support for serializing std::byte C-+style arrays as a side effect.

refactor: Print verbose serialize compiler error messages

https://github.com/bitcoin/bitcoin/pull/28423/commits/37e2b011136ca1cf00dfb9e575d12f0d035a6a2c
https://github.com/bitcoin/bitcoin/pull/29056/commits/fa898e6836a8fc2c7b6c8c15ad21818b16a89863

Allow std::byte C-style array serialization

https://github.com/bitcoin/bitcoin/pull/29056/commits/fae526345de539ab8f9b80100f6dfbe8e1d3284b

serialization: replace char-is-int8_t autoconf detection with c++20 concept

https://github.com/bitcoin/bitcoin/pull/29484/commits/ad7584d8b60119ca3717117a1eb6a16d753c5d74

This is a backport of core#29056 and core#29484
With also one commit from core#28423

Depends on D19039

Test Plan

ninja all check-all

Event Timeline

PiRK requested review of this revision.Wed, Dec 3, 15:56

Tail of the build log:

tests/test_iguana.py::test_redeem_script_empty_stack ##teamcity[testStarted timestamp='2025-12-03T16:03:33.708' captureStandardOutput='false' flowId='tests.test_iguana.test_redeem_script_empty_stack' metainfo='test_redeem_script_empty_stack' name='tests.test_iguana.test_redeem_script_empty_stack']
PASSED              [ 85%]##teamcity[testFinished timestamp='2025-12-03T16:03:33.720' duration='11' flowId='tests.test_iguana.test_redeem_script_empty_stack' name='tests.test_iguana.test_redeem_script_empty_stack']

tests/test_iguana.py::test_redeem_script_false ##teamcity[testStarted timestamp='2025-12-03T16:03:33.721' captureStandardOutput='false' flowId='tests.test_iguana.test_redeem_script_false' metainfo='test_redeem_script_false' name='tests.test_iguana.test_redeem_script_false']
PASSED                    [ 90%]##teamcity[testFinished timestamp='2025-12-03T16:03:33.732' duration='11' flowId='tests.test_iguana.test_redeem_script_false' name='tests.test_iguana.test_redeem_script_false']

tests/test_iguana.py::test_redeem_script_cleanstack ##teamcity[testStarted timestamp='2025-12-03T16:03:33.733' captureStandardOutput='false' flowId='tests.test_iguana.test_redeem_script_cleanstack' metainfo='test_redeem_script_cleanstack' name='tests.test_iguana.test_redeem_script_cleanstack']
PASSED               [ 95%]##teamcity[testFinished timestamp='2025-12-03T16:03:33.745' duration='11' flowId='tests.test_iguana.test_redeem_script_cleanstack' name='tests.test_iguana.test_redeem_script_cleanstack']

tests/test_iguana.py::test_redeem_script_input_sigchecks ##teamcity[testStarted timestamp='2025-12-03T16:03:33.746' captureStandardOutput='false' flowId='tests.test_iguana.test_redeem_script_input_sigchecks' metainfo='test_redeem_script_input_sigchecks' name='tests.test_iguana.test_redeem_script_input_sigchecks']
PASSED          [100%]##teamcity[testFinished timestamp='2025-12-03T16:03:33.773' duration='26' flowId='tests.test_iguana.test_redeem_script_input_sigchecks' name='tests.test_iguana.test_redeem_script_input_sigchecks']


============================== 20 passed in 0.62s ==============================
[221/540] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/guiutiltests.cpp.o
[222/540] Building CXX object src/pow/test/CMakeFiles/test-pow.dir/fixture.cpp.o
[223/540] Linking CXX executable src/pow/test/test-pow
[224/540] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/apptests.cpp.o
[225/540] pow: testing daa_tests
[226/540] pow: testing eda_tests
[227/540] Running utility command for check-pow-eda_tests
[228/540] Running utility command for check-pow-daa_tests
[229/540] pow: testing grasberg_tests
[230/540] Running utility command for check-pow-grasberg_tests
[231/540] Test Bitcoin utilities...
[232/540] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/uritests.cpp.o
[233/540] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/optiontests.cpp.o
[234/540] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/test_main.cpp.o
[235/540] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/rpcnestedtests.cpp.o
[236/540] pow: testing aserti32d_tests
[237/540] Running utility command for check-pow-aserti32d_tests
[238/540] Running pow test suite
PASSED: pow test suite
[239/540] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/fixture.cpp.o
[240/540] Linking CXX executable src/seeder/test/test-seeder
[241/540] seeder: testing parse_name_tests
[242/540] seeder: testing message_writer_tests
[243/540] seeder: testing db_tests
[244/540] seeder: testing options_tests
[245/540] seeder: testing p2p_messaging_tests
[246/540] seeder: testing write_name_tests
[247/540] Running utility command for check-seeder-parse_name_tests
[248/540] Running utility command for check-seeder-message_writer_tests
[249/540] Running utility command for check-seeder-options_tests
[250/540] Running utility command for check-seeder-db_tests
[251/540] Running utility command for check-seeder-p2p_messaging_tests
[252/540] Running utility command for check-seeder-write_name_tests
[253/540] Running seeder test suite
PASSED: seeder test suite
[254/540] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/paymentservertests.cpp.o
[255/540] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/__/__/wallet/test/wallet_test_fixture.cpp.o
[256/540] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/addressbooktests.cpp.o
[257/540] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/wallettests.cpp.o
[258/540] Linking CXX executable src/qt/test/test_bitcoin-qt
[259/540] bitcoin-qt: testing test_bitcoin-qt
[260/540] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang failed with exit code 1

Tail of the build log:

[205/545] Automatic MOC for target test_bitcoin-qt
[206/545] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/fixture.cpp.o
[207/545] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/write_name_tests.cpp.o
[208/545] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/__/__/chronik/test/chronikbridge_tests.cpp.o
[209/545] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/options_tests.cpp.o
[210/545] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/db_tests.cpp.o
[211/545] Test Bitcoin utilities...
[212/545] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/__/wallet/test/wallet_tests.cpp.o
[213/545] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/message_writer_tests.cpp.o
[214/545] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/__/__/chronik/test/bridgeprimitives_tests.cpp.o
[215/545] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/p2p_messaging_tests.cpp.o
[216/545] Building CXX object src/pow/test/CMakeFiles/test-pow.dir/daa_tests.cpp.o
[217/545] Linking CXX executable src/seeder/test/test-seeder
[218/545] seeder: testing db_tests
[219/545] Running utility command for check-seeder-db_tests
[220/545] seeder: testing message_writer_tests
[221/545] Running utility command for check-seeder-message_writer_tests
[222/545] seeder: testing options_tests
[223/545] Running utility command for check-seeder-options_tests
[224/545] seeder: testing p2p_messaging_tests
[225/545] Running utility command for check-seeder-p2p_messaging_tests
[226/545] seeder: testing parse_name_tests
[227/545] Building CXX object src/pow/test/CMakeFiles/test-pow.dir/eda_tests.cpp.o
[228/545] Running utility command for check-seeder-parse_name_tests
[229/545] seeder: testing write_name_tests
[230/545] Running utility command for check-seeder-write_name_tests
[231/545] Running seeder test suite
PASSED: seeder test suite
[232/545] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/util.cpp.o
[233/545] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/bitcoinaddressvalidatortests.cpp.o
[234/545] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/compattests.cpp.o
[235/545] Building CXX object src/pow/test/CMakeFiles/test-pow.dir/grasberg_tests.cpp.o
[236/545] Linking CXX executable src/pow/test/test-pow
[237/545] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/test_bitcoin-qt_autogen/mocs_compilation.cpp.o
[238/545] pow: testing daa_tests
[239/545] Running utility command for check-pow-daa_tests
[240/545] pow: testing eda_tests
[241/545] Running utility command for check-pow-eda_tests
[242/545] pow: testing grasberg_tests
[243/545] Running utility command for check-pow-grasberg_tests
[244/545] pow: testing aserti32d_tests
[245/545] Running utility command for check-pow-aserti32d_tests
[246/545] Running pow test suite
PASSED: pow test suite
[247/545] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/optiontests.cpp.o
[248/545] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/guiutiltests.cpp.o
[249/545] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/uritests.cpp.o
[250/545] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/apptests.cpp.o
[251/545] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/rpcnestedtests.cpp.o
[252/545] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/test_main.cpp.o
[253/545] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/__/__/wallet/test/wallet_test_fixture.cpp.o
[254/545] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/paymentservertests.cpp.o
[255/545] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/addressbooktests.cpp.o
[256/545] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/wallettests.cpp.o
[257/545] Linking CXX executable src/qt/test/test_bitcoin-qt
[258/545] bitcoin-qt: testing test_bitcoin-qt
[259/545] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang-tidy failed with exit code 1
PiRK planned changes to this revision.Wed, Dec 3, 16:14

should have checked clang before submitting

In D19040#433674, @PiRK wrote:

should have checked clang before submitting

Can't reproduce, even with clang, but I hope core#23477 can solve this by removing the erroring line of code