Merge bitcoin/bitcoin#34639: iwyu: Document or remove some pragma: export and other improvements

0fe6fccec2 doc: Document rationale for using `IWYU pragma: export` (Hennadii Stepanov)
cfa3b10d50 iwyu, doc: Document `IWYU pragma: export` for `<logging/categories.h>` (Hennadii Stepanov)
015bea05e6 iwyu, doc: Document `IWYU pragma: export` for `<chrono>` (Hennadii Stepanov)
48bfcfedec iwyu, doc: Document `IWYU pragma: export` for `<threadsafety.h>` (Hennadii Stepanov)
179abb387f refactor: Move `StdMutex` to its own header (Hennadii Stepanov)
6d2952c3c3 serialize: Add missing `<span>` header (Hennadii Stepanov)

Pull request description:

  This PR is a prerequisite for https://github.com/bitcoin/bitcoin/pull/34448. It was split into a separate PR to limit the scope and minimize potential merge conflicts.

  The first commit improves the accuracy of IWYU suggestions within our heavily templated code. Note that, for now, the `serialize.h` header itself is excluded from IWYU inspection because it lacks a corresponding source file.

  The remaining commits follow the Developer Notes [guidance](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#using-iwyu):
  > Use `IWYU pragma: export` very sparingly, as this enforces transitive inclusion of headers and undermines the specific purpose of IWYU.

ACKs for top commit:
  maflcko:
    review ACK 0fe6fccec2 👤
  ajtowns:
    utACK 0fe6fccec2

Tree-SHA512: dc2d4e3ff78b9707a1a26cb9b1c0a456de0d33c59e773bbf692344c2fceaff8936317479c5e898038f29134bc0e5d9d1ef7350e53512dd8e262f46ede578c4f9

doc/developer-notes.md

@@ -542,6 +542,13 @@ to a function that accepts a `std::string` parameter. An implicit conversion occ
 Use `IWYU pragma: export` very sparingly, as this enforces transitive inclusion of headers
 and undermines the specific purpose of IWYU.
 
+The acceptable cases for using `IWYU pragma: export` are:
+1. Facade headers. For example, see [`compat/compat.h`](/src/compat/compat.h).
+2. Drop-in replacement headers. For example, see [`util/time.h`](/src/util/time.h).
+3. Presenting a complete interface across multiple headers.
+
+A comment explaining the rationale is required for every use of `IWYU pragma: export`.
+
 ### Performance profiling with perf
 
 Profiling is a good way to get a precise idea of where time is being spent in

src/index/base.cpp

@@ -35,7 +35,6 @@
 #include <cstdint>
 #include <memory>
 #include <optional>
-#include <span>
 #include <stdexcept>
 #include <string>
 #include <thread>

src/index/base.h

@@ -9,7 +9,7 @@
 #include <dbwrapper.h>
 #include <interfaces/chain.h>
 #include <kernel/cs_main.h>
-#include <threadsafety.h>
+#include <sync.h>
 #include <uint256.h>
 #include <util/fs.h>
 #include <util/threadinterrupt.h>

src/index/blockfilterindex.cpp

@@ -28,7 +28,6 @@
 #include <exception>
 #include <map>
 #include <optional>
-#include <span>
 #include <stdexcept>
 #include <string>
 #include <tuple>

src/index/coinstatsindex.cpp

@@ -30,7 +30,6 @@
 
 #include <compare>
 #include <limits>
-#include <span>
 #include <string>
 #include <utility>
 #include <vector>

src/index/txindex.cpp

@@ -24,7 +24,6 @@
 #include <cstdint>
 #include <cstdio>
 #include <exception>
-#include <span>
 #include <string>
 #include <utility>
 #include <vector>

src/index/txospenderindex.cpp

@@ -26,7 +26,6 @@
 #include <cstdio>
 #include <exception>
 #include <ios>
-#include <span>
 #include <string>
 #include <utility>
 #include <vector>

src/kernel/coinstats.cpp

@@ -22,7 +22,6 @@
 
 #include <map>
 #include <memory>
-#include <span>
 #include <utility>
 #include <version>
 

src/kernel/mempool_entry.h

@@ -13,8 +13,8 @@
 #include <primitives/transaction.h>
 #include <txgraph.h>
 #include <util/overflow.h>
+#include <util/time.h>
 
-#include <chrono>
 #include <cstddef>
 #include <cstdint>
 #include <functional>

src/kernel/mempool_options.h

@@ -8,8 +8,8 @@
 
 #include <policy/feerate.h>
 #include <policy/policy.h>
+#include <util/time.h>
 
-#include <chrono>
 #include <cstdint>
 #include <optional>
 

src/logging.h

@@ -8,9 +8,9 @@
 
 #include <crypto/siphash.h>
 #include <logging/categories.h> // IWYU pragma: export
-#include <threadsafety.h>
 #include <util/fs.h>
 #include <util/log.h> // IWYU pragma: export
+#include <util/stdmutex.h>
 #include <util/string.h>
 #include <util/time.h>
 

src/net_processing.h

@@ -11,7 +11,6 @@
 #include <node/txorphanage.h>
 #include <private_broadcast.h>
 #include <protocol.h>
-#include <threadsafety.h>
 #include <uint256.h>
 #include <util/expected.h>
 #include <validationinterface.h>

src/node/chainstate.cpp

@@ -11,7 +11,6 @@
 #include <kernel/caches.h>
 #include <node/blockstorage.h>
 #include <sync.h>
-#include <threadsafety.h>
 #include <tinyformat.h>
 #include <txdb.h>
 #include <uint256.h>

src/node/kernel_notifications.h

@@ -8,7 +8,6 @@
 #include <kernel/notifications_interface.h>
 
 #include <sync.h>
-#include <threadsafety.h>
 #include <uint256.h>
 
 #include <atomic>

src/node/utxo_snapshot.cpp

@@ -15,7 +15,6 @@
 #include <cassert>
 #include <cstdio>
 #include <optional>
-#include <span>
 #include <string>
 
 namespace node {

src/policy/fees/block_policy_estimator.h

@@ -9,7 +9,6 @@
 #include <policy/feerate.h>
 #include <random.h>
 #include <sync.h>
-#include <threadsafety.h>
 #include <uint256.h>
 #include <util/fs.h>
 #include <validationinterface.h>

src/policy/rbf.h

@@ -7,7 +7,7 @@
 
 #include <consensus/amount.h>
 #include <primitives/transaction.h>
-#include <threadsafety.h>
+#include <sync.h>
 #include <txmempool.h>
 #include <util/feefrac.h>
 

src/primitives/block.cpp

@@ -9,7 +9,6 @@
 #include <tinyformat.h>
 
 #include <memory>
-#include <span>
 #include <sstream>
 
 uint256 CBlockHeader::GetHash() const

src/private_broadcast.h

@@ -9,7 +9,6 @@
 #include <primitives/transaction.h>
 #include <primitives/transaction_identifier.h>
 #include <sync.h>
-#include <threadsafety.h>
 #include <util/time.h>
 
 #include <optional>

src/rpc/blockchain.h

@@ -9,7 +9,6 @@
 #include <core_io.h>
 #include <streams.h>
 #include <sync.h>
-#include <threadsafety.h>
 #include <util/fs.h>
 #include <validation.h>
 

src/scheduler.h

@@ -7,7 +7,6 @@
 
 #include <attributes.h>
 #include <sync.h>
-#include <threadsafety.h>
 #include <util/task_runner.h>
 
 #include <chrono>

src/serialize.h

@@ -22,6 +22,7 @@
 #include <map>
 #include <memory>
 #include <set>
+#include <span>
 #include <string>
 #include <utility>
 #include <vector>

src/sync.h

@@ -6,6 +6,8 @@
 #ifndef BITCOIN_SYNC_H
 #define BITCOIN_SYNC_H
 
+// This header declares threading primitives compatible with Clang
+// Thread Safety Analysis and provides appropriate annotation macros.
 #include <threadsafety.h> // IWYU pragma: export
 #include <util/macros.h>
 

src/test/fuzz/util/net.h

@@ -13,10 +13,10 @@
 #include <node/connection_types.h>
 #include <node/eviction.h>
 #include <protocol.h>
+#include <sync.h>
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/util.h>
 #include <test/util/net.h>
-#include <threadsafety.h>
 #include <util/asmap.h>
 #include <util/sock.h>
 

src/threadsafety.h

@@ -6,8 +6,6 @@
 #ifndef BITCOIN_THREADSAFETY_H
 #define BITCOIN_THREADSAFETY_H
 
-#include <mutex>
-
 #ifdef __clang__
 // TL;DR Add GUARDED_BY(mutex) to member variables. The others are
 // rarely necessary. Ex: int nFoo GUARDED_BY(cs_foo);
@@ -54,26 +52,4 @@
 #define ASSERT_EXCLUSIVE_LOCK(...)
 #endif // __GNUC__
 
-// StdMutex provides an annotated version of std::mutex for us,
-// and should only be used when sync.h Mutex/LOCK/etc are not usable.
-class LOCKABLE StdMutex : public std::mutex
-{
-public:
-#ifdef __clang__
-    //! For negative capabilities in the Clang Thread Safety Analysis.
-    //! A negative requirement uses the EXCLUSIVE_LOCKS_REQUIRED attribute, in conjunction
-    //! with the ! operator, to indicate that a mutex should not be held.
-    const StdMutex& operator!() const { return *this; }
-#endif // __clang__
-};
-
-// StdLockGuard provides an annotated version of std::lock_guard for us,
-// and should only be used when sync.h Mutex/LOCK/etc are not usable.
-class SCOPED_LOCKABLE StdLockGuard : public std::lock_guard<StdMutex>
-{
-public:
-    explicit StdLockGuard(StdMutex& cs) EXCLUSIVE_LOCK_FUNCTION(cs) : std::lock_guard<StdMutex>(cs) {}
-    ~StdLockGuard() UNLOCK_FUNCTION() = default;
-};
-
 #endif // BITCOIN_THREADSAFETY_H

src/util/log.h

@@ -5,6 +5,8 @@
 #ifndef BITCOIN_UTIL_LOG_H
 #define BITCOIN_UTIL_LOG_H
 
+// This header works in tandem with `logging/categories.h`
+// to expose the complete logging interface.
 #include <logging/categories.h> // IWYU pragma: export
 #include <tinyformat.h>
 #include <util/check.h>

src/util/stdmutex.h

@@ -0,0 +1,37 @@
+// Copyright (c) 2009-2010 Satoshi Nakamoto
+// Copyright (c) 2009-present The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_UTIL_STDMUTEX_H
+#define BITCOIN_UTIL_STDMUTEX_H
+
+// This header declares threading primitives compatible with Clang
+// Thread Safety Analysis and provides appropriate annotation macros.
+#include <threadsafety.h> // IWYU pragma: export
+
+#include <mutex>
+
+// StdMutex provides an annotated version of std::mutex for us,
+// and should only be used when sync.h Mutex/LOCK/etc are not usable.
+class LOCKABLE StdMutex : public std::mutex
+{
+public:
+#ifdef __clang__
+    //! For negative capabilities in the Clang Thread Safety Analysis.
+    //! A negative requirement uses the EXCLUSIVE_LOCKS_REQUIRED attribute, in conjunction
+    //! with the ! operator, to indicate that a mutex should not be held.
+    const StdMutex& operator!() const { return *this; }
+#endif // __clang__
+};
+
+// StdLockGuard provides an annotated version of std::lock_guard for us,
+// and should only be used when sync.h Mutex/LOCK/etc are not usable.
+class SCOPED_LOCKABLE StdLockGuard : public std::lock_guard<StdMutex>
+{
+public:
+    explicit StdLockGuard(StdMutex& cs) EXCLUSIVE_LOCK_FUNCTION(cs) : std::lock_guard<StdMutex>(cs) {}
+    ~StdLockGuard() UNLOCK_FUNCTION() = default;
+};
+
+#endif // BITCOIN_UTIL_STDMUTEX_H

src/util/threadinterrupt.h

@@ -6,7 +6,6 @@
 #define BITCOIN_UTIL_THREADINTERRUPT_H
 
 #include <sync.h>
-#include <threadsafety.h>
 
 #include <atomic>
 #include <chrono>

src/util/time.h

@@ -6,6 +6,7 @@
 #ifndef BITCOIN_UTIL_TIME_H
 #define BITCOIN_UTIL_TIME_H
 
+// The `util/time.h` header is designed to be a drop-in replacement for `chrono`.
 #include <chrono> // IWYU pragma: export
 #include <cstdint>
 #include <optional>

src/zmq/zmqpublishnotifier.cpp

@@ -24,7 +24,6 @@
 #include <cstring>
 #include <map>
 #include <optional>
-#include <span>
 #include <string>
 #include <utility>
 #include <vector>