✅ fanquake closed an issue: "Check usages of `#if defined(...)`"
(https://github.com/bitcoin/bitcoin/issues/16419)

💬 fanquake commented on issue "Check usages of `#if defined(...)`":
(https://github.com/bitcoin/bitcoin/issues/16419#issuecomment-2186722083)
Going to close this now that #29876 is merged.

🤔 theuni reviewed a pull request: "ci: Use C++23 in one task"
(https://github.com/bitcoin/bitcoin/pull/30735#pullrequestreview-2266924928)
Concept ACK.

If we wanted, we could actually start conditionally enabling c++23 features for these builds. For byteswap, something like:
```c++
#if __cplusplus >= 202302L && defined(__cpp_lib_byteswap) && __cpp_lib_byteswap >= 202110L
# define bitcoin_builtin_bswap16(x) std::byteswap(x)
# define bitcoin_builtin_bswap32(x) std::byteswap(x)
# define bitcoin_builtin_bswap64(x) std::byteswap(x)
# elif defined __has_builtin
...
```

Another I've found useful while working
...

💬 TheCharlatan commented on pull request "build: work around issue with Boost <= 1.80 and Clang >= 18":
(https://github.com/bitcoin/bitcoin/pull/30821#discussion_r1745569113)
Should this be in a `#if defined(__clang__)` block?

💬 theStack commented on pull request "wallet: optimize migration process, batch db transactions":
(https://github.com/bitcoin/bitcoin/pull/28574#discussion_r1774150203)
preprocessor nit, as one-liner:
```suggestion
#if defined(USE_BDB) && defined(USE_SQLITE)
```

💬 promag commented on pull request "qt6, test: Handle deprecated code":
(https://github.com/bitcoin-core/gui/pull/839#discussion_r1786676537)
2b5fd8fd24026ecf2fac7af6657e2363bfe3706c

An alternative to reduce duplicate code and the diff is to define `QVERIFY_EXCEPTION_THROWN`, something like?

```cpp
#if (QT_VERSION >= QT_VERSION_CHECK(6, 3, 0))
#define QVERIFY_EXCEPTION_THROWN(a, b) QVERIFY_THROWS_EXCEPTION(b, a)
#endif
```

💬 hebasto commented on pull request "qt6: Handle different signatures of `QANEF::nativeEventFilter`":
(https://github.com/bitcoin-core/gui/pull/840#issuecomment-2393637703)
> If Qt 6.7.0 is going to be required for Windows after the switchover; why change this code now just to immediately drop it after migrating to Qt 6?

To make https://github.com/bitcoin/bitcoin/pull/30997 focused on the build system changes only.

We already have:
```
$ git grep 'QT_VERSION_CHECK(6, 0, 0)'
src/qt/addressbookpage.cpp:#if (QT_VERSION >= QT_VERSION_CHECK(6, 0, 0))
src/qt/addressbookpage.cpp:#if (QT_VERSION >= QT_VERSION_CHECK(6, 0, 0))
src/qt/bitcoin.cpp:#if (QT_VERSION <
...

🤔 promag reviewed a pull request: "qt6: Handle different signatures of `QANEF::nativeEventFilter`"
(https://github.com/bitcoin-core/gui/pull/840#pullrequestreview-2349357223)
I think small changes to make code compatible with both versions are fine until support for qt5 is dropped.

Code review ACK 80761afced12c774a1768fb27a3975d456981ae0.

Alternative is to use type alias
```cpp
#if (QT_VERSION >= QT_VERSION_CHECK(6, 0, 0))
using result_t = qintptr*;
#else
using result_t = long*;
#endif

💬 stickies-v commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r1849132153)
Sorry, I forgot to include the diff which updates the macro (and 1 instance to show it compiles):

<details>
<summary>git diff on 6c9121f790</summary>

```diff
diff --git a/src/kernel/bitcoinkernel.h b/src/kernel/bitcoinkernel.h
index 9e6bf127db..67248349e2 100644
--- a/src/kernel/bitcoinkernel.h
+++ b/src/kernel/bitcoinkernel.h
@@ -31,9 +31,9 @@
#define BITCOINKERNEL_WARN_UNUSED_RESULT
#endif
#if !defined(BITCOINKERNEL_BUILD) && defined(__GNUC__) && BITCOINKERNEL_GNUC_PREREQ(3,
...

💬 hodlinator commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1880956056)
> https://godbolt.org/z/6336nGKcM (replace `0` with `1` to enable the compile-time checks)) into a compile failure on all compilers seems better than a runtime error, possibly only in CI.

With the `#if` set to `1` and lines 42-43 changed to
```C++
check = RuntimeFormat("fmt");
```
One still gets an ASAN error instead of a compile time error.