diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index daeefddcb609..f54e06616f76 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -258,7 +258,7 @@ jobs: sed -i '1s/^/set(ENV{CMAKE_POLICY_VERSION_MINIMUM} 3.5)\n/' "${VCPKG_INSTALLATION_ROOT}/scripts/ports.cmake" - name: vcpkg tools cache - uses: actions/cache@v4 + uses: actions/cache@v5 with: path: C:/vcpkg/downloads/tools key: ${{ github.job }}-vcpkg-tools @@ -409,7 +409,7 @@ jobs: uses: ./.github/actions/save-caches - name: Upload built executables - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v6 with: name: ${{ matrix.artifact-name }}-${{ github.run_id }} path: | @@ -447,7 +447,7 @@ jobs: ref: ${{ needs.record-frozen-commit.outputs.commit }} - name: Download built executables - uses: actions/download-artifact@v5 + uses: actions/download-artifact@v7 with: name: ${{ matrix.artifact-name }}-${{ github.run_id }} diff --git a/ci/test/01_iwyu.patch b/ci/test/01_iwyu.patch index e14316bfb3c6..e7d75f4ee7ae 100644 --- a/ci/test/01_iwyu.patch +++ b/ci/test/01_iwyu.patch @@ -539,10 +539,11 @@ See: https://github.com/include-what-you-use/include-what-you-use/blob/clang_21/ }; const IncludeMapEntry stdlib_c_include_map[] = { -@@ -601,31 +601,31 @@ const IncludeMapEntry stdlib_c_include_map[] = { +@@ -600,32 +600,32 @@ const IncludeMapEntry stdlib_c_include_map[] = { + // https://github.com/cplusplus/draft/blob/c+%2B20/source/lib-intro.tex // // $ curl -s -N https://raw.githubusercontent.com/cplusplus/draft/c%2B%2B20/source/lib-intro.tex | sed -n '/begin{multicolfloattable}.*{headers.cpp.c}/,/end{multicolfloattable}/p' | grep tcode | perl -nle 'm/tcode{}/ && print qq@ { "<$1.h>", kPublic, "", kPublic },@' | sort - { "", kPublic, "", kPublic }, +- { "", kPublic, "", kPublic }, - { "", kPublic, "", kPublic }, - { "", kPublic, "", kPublic }, - { "", kPublic, "", kPublic }, @@ -568,6 +569,7 @@ See: https://github.com/include-what-you-use/include-what-you-use/blob/clang_21/ - { "", kPublic, "", kPublic }, - { "", kPublic, "", kPublic }, - { "", kPublic, "", kPublic }, ++ { "", kPrivate, "", kPublic }, + { "", kPrivate, "", kPublic }, + { "", kPrivate, "", kPublic }, + { "", kPrivate, "", kPublic }, diff --git a/ci/test/03_test_script.sh b/ci/test/03_test_script.sh index 1bd9fcfc69b0..050d9e6a5999 100755 --- a/ci/test/03_test_script.sh +++ b/ci/test/03_test_script.sh @@ -215,7 +215,7 @@ fi if [[ "${RUN_IWYU}" == true ]]; then # TODO: Consider enforcing IWYU across the entire codebase. - FILES_WITH_ENFORCED_IWYU="/src/((crypto|index)/.*\\.cpp|node/blockstorage.cpp|node/utxo_snapshot.cpp|core_read.cpp|signet.cpp|kernel/chain.cpp)" + FILES_WITH_ENFORCED_IWYU="/src/((crypto|index|kernel)/.*\\.cpp|node/blockstorage.cpp|node/utxo_snapshot.cpp|core_io.cpp|signet.cpp)" jq --arg patterns "$FILES_WITH_ENFORCED_IWYU" 'map(select(.file | test($patterns)))' "${BASE_BUILD_DIR}/compile_commands.json" > "${BASE_BUILD_DIR}/compile_commands_iwyu_errors.json" jq --arg patterns "$FILES_WITH_ENFORCED_IWYU" 'map(select(.file | test($patterns) | not))' "${BASE_BUILD_DIR}/compile_commands.json" > "${BASE_BUILD_DIR}/compile_commands_iwyu_warnings.json" diff --git a/contrib/devtools/circular-dependencies.py b/contrib/devtools/circular-dependencies.py index af19b3a05efa..7c0581717308 100755 --- a/contrib/devtools/circular-dependencies.py +++ b/contrib/devtools/circular-dependencies.py @@ -6,11 +6,6 @@ import sys import re -MAPPING = { - 'core_read.cpp': 'core_io.cpp', - 'core_write.cpp': 'core_io.cpp', -} - # Directories with header-based modules, where the assumption that .cpp files # define functions and variables declared in corresponding .h files is # incorrect. @@ -19,8 +14,6 @@ ] def module_name(path): - if path in MAPPING: - path = MAPPING[path] if any(path.startswith(dirpath) for dirpath in HEADER_MODULE_PATHS): return path if path.endswith(".h"): diff --git a/doc/developer-notes.md b/doc/developer-notes.md index d17f8024ee48..b8a16ec09016 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -558,6 +558,21 @@ llvm-cov show \ The generated coverage report can be accessed at `build/coverage_report/index.html`. +### Using IWYU + +The [`include-what-you-use`](https://github.com/include-what-you-use/include-what-you-use) tool (IWYU) +helps to enforce the source code organization [policy](#source-code-organization) in this repository. + +To ensure consistency, it is recommended to run the IWYU CI job locally rather than running the tool directly. + +In some cases, IWYU might suggest headers that seem unnecessary at first glance, but are actually required. +For example, a macro may use a symbol that requires its own include. Another example is passing a string literal +to a function that accepts a `std::string` parameter. An implicit conversion occurs at the callsite using the +`std::string` constructor, which makes the corresponding header required. We accept these suggestions as is. + +Use `IWYU pragma: export` very sparingly, as this enforces transitive inclusion of headers +and undermines the specific purpose of IWYU. + ### Performance profiling with perf Profiling is a good way to get a precise idea of where time is being spent in @@ -1057,7 +1072,7 @@ Write scripts in Python or Rust rather than bash, when possible. - *Rationale*: Excluding headers because they are already indirectly included results in compilation failures when those indirect dependencies change. Furthermore, it obscures what the real code - dependencies are. + dependencies are. The [Using IWYU](#using-iwyu) section describes a tool to help enforce this. - Don't import anything into the global namespace (`using namespace ...`). Use fully specified types such as `std::string`. diff --git a/src/.clang-format b/src/.clang-format index 4db091385563..2b74e40b41dc 100644 --- a/src/.clang-format +++ b/src/.clang-format @@ -142,6 +142,7 @@ LambdaBodyIndentation: Signature LineEnding: DeriveLF MacroBlockBegin: '' MacroBlockEnd: '' +MainIncludeChar: AngleBracket MaxEmptyLinesToKeep: 2 NamespaceIndentation: None ObjCBinPackProtocolList: Auto diff --git a/src/.clang-tidy b/src/.clang-tidy index 011536498e1b..a4378cea308f 100644 --- a/src/.clang-tidy +++ b/src/.clang-tidy @@ -24,6 +24,7 @@ performance-*, -performance-no-int-to-ptr, -performance-noexcept-move-constructor, -performance-unnecessary-value-param, +readability-avoid-const-params-in-decls, readability-const-return-type, readability-container-contains, readability-redundant-declaration, diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index a1e0c5f131a5..cf1f26c9f24c 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -106,8 +106,7 @@ add_library(bitcoin_common STATIC EXCLUDE_FROM_ALL common/system.cpp common/url.cpp compressor.cpp - core_read.cpp - core_write.cpp + core_io.cpp deploymentinfo.cpp external_signer.cpp init/common.cpp diff --git a/src/addrman.h b/src/addrman.h index 3a3230143c19..8368e30b7b76 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -172,7 +172,7 @@ class AddrMan * * @return A vector of randomly selected addresses from vRandom. */ - std::vector GetAddr(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered = true) const; + std::vector GetAddr(size_t max_addresses, size_t max_pct, std::optional network, bool filtered = true) const; /** * Returns an information-location pair for all addresses in the selected addrman table. diff --git a/src/addrman_impl.h b/src/addrman_impl.h index f825d20b032c..c1716f891d62 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -135,7 +135,7 @@ class AddrManImpl std::pair Select(bool new_only, const std::unordered_set& networks) const EXCLUSIVE_LOCKS_REQUIRED(!cs); - std::vector GetAddr(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered = true) const + std::vector GetAddr(size_t max_addresses, size_t max_pct, std::optional network, bool filtered = true) const EXCLUSIVE_LOCKS_REQUIRED(!cs); std::vector> GetEntries(bool from_tried) const @@ -267,7 +267,7 @@ class AddrManImpl * */ nid_type GetEntry(bool use_tried, size_t bucket, size_t position) const EXCLUSIVE_LOCKS_REQUIRED(cs); - std::vector GetAddr_(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered = true) const EXCLUSIVE_LOCKS_REQUIRED(cs); + std::vector GetAddr_(size_t max_addresses, size_t max_pct, std::optional network, bool filtered = true) const EXCLUSIVE_LOCKS_REQUIRED(cs); std::vector> GetEntries_(bool from_tried) const EXCLUSIVE_LOCKS_REQUIRED(cs); diff --git a/src/chainparams.h b/src/chainparams.h index 571e9e177b2b..732949e82f69 100644 --- a/src/chainparams.h +++ b/src/chainparams.h @@ -15,7 +15,7 @@ class ArgsManager; /** * Creates and returns a std::unique_ptr of the chosen chain. */ -std::unique_ptr CreateChainParams(const ArgsManager& args, const ChainType chain); +std::unique_ptr CreateChainParams(const ArgsManager& args, ChainType chain); /** * Return the currently selected parameters. This won't change after app @@ -26,6 +26,6 @@ const CChainParams &Params(); /** * Sets the params returned by Params() to those for the given chain type. */ -void SelectParams(const ChainType chain); +void SelectParams(ChainType chain); #endif // BITCOIN_CHAINPARAMS_H diff --git a/src/chainparamsbase.h b/src/chainparamsbase.h index 37e69b3285fc..1407f196f3b8 100644 --- a/src/chainparamsbase.h +++ b/src/chainparamsbase.h @@ -35,7 +35,7 @@ class CBaseChainParams /** * Creates and returns a std::unique_ptr of the chosen chain. */ -std::unique_ptr CreateBaseChainParams(const ChainType chain); +std::unique_ptr CreateBaseChainParams(ChainType chain); /** *Set the arguments for chainparams @@ -49,7 +49,7 @@ void SetupChainParamsBaseOptions(ArgsManager& argsman); const CBaseChainParams& BaseParams(); /** Sets the params returned by Params() to those for the given chain. */ -void SelectBaseParams(const ChainType chain); +void SelectBaseParams(ChainType chain); /** List of possible chain / network names */ #define LIST_CHAIN_NAMES "main, test, testnet4, signet, regtest" diff --git a/src/common/bloom.h b/src/common/bloom.h index bff96a23a37a..97007e1ff517 100644 --- a/src/common/bloom.h +++ b/src/common/bloom.h @@ -61,7 +61,7 @@ class CBloomFilter * It should generally always be a random value (and is largely only exposed for unit testing) * nFlags should be one of the BLOOM_UPDATE_* enums (not _MASK) */ - CBloomFilter(const unsigned int nElements, const double nFPRate, const unsigned int nTweak, unsigned char nFlagsIn); + CBloomFilter(unsigned int nElements, double nFPRate, unsigned int nTweak, unsigned char nFlagsIn); CBloomFilter() : nHashFuncs(0), nTweak(0), nFlags(0) {} SERIALIZE_METHODS(CBloomFilter, obj) { READWRITE(obj.vData, obj.nHashFuncs, obj.nTweak, obj.nFlags); } @@ -108,7 +108,7 @@ class CBloomFilter class CRollingBloomFilter { public: - CRollingBloomFilter(const unsigned int nElements, const double nFPRate); + CRollingBloomFilter(unsigned int nElements, double nFPRate); void insert(std::span vKey); bool contains(std::span vKey) const; diff --git a/src/common/messages.h b/src/common/messages.h index 4cabdc79f4df..60fdaa18625e 100644 --- a/src/common/messages.h +++ b/src/common/messages.h @@ -31,7 +31,7 @@ std::string FeeModeInfo(std::pair& mode); std::string FeeModesDetail(std::string default_info); std::string InvalidEstimateModeErrorMessage(); bilingual_str PSBTErrorString(PSBTError error); -bilingual_str TransactionErrorString(const node::TransactionError error); +bilingual_str TransactionErrorString(node::TransactionError error); bilingual_str ResolveErrMsg(const std::string& optname, const std::string& strBind); bilingual_str InvalidPortErrMsg(const std::string& optname, const std::string& strPort); bilingual_str AmountHighWarn(const std::string& optname); diff --git a/src/common/signmessage.h b/src/common/signmessage.h index 4533875a757b..c2152f353369 100644 --- a/src/common/signmessage.h +++ b/src/common/signmessage.h @@ -72,6 +72,6 @@ bool MessageSign( */ uint256 MessageHash(const std::string& message); -std::string SigningResultString(const SigningResult res); +std::string SigningResultString(SigningResult res); #endif // BITCOIN_COMMON_SIGNMESSAGE_H diff --git a/src/core_write.cpp b/src/core_io.cpp similarity index 54% rename from src/core_write.cpp rename to src/core_io.cpp index e4f23458c81e..e715a329e349 100644 --- a/src/core_write.cpp +++ b/src/core_io.cpp @@ -4,25 +4,286 @@ #include -#include +#include +#include #include #include #include +#include #include +// IWYU incorrectly suggests replacing this header +// with forward declarations. +// See https://github.com/include-what-you-use/include-what-you-use/issues/1886. +#include // IWYU pragma: keep +#include #include