Modernize Codebase Using C++20 Features

[drebelsky]

Based on #5091. Clean up codebase using C++20 features, resolves #4673.

• Replace BUCKET_TYPE_ASSERT with IsBucketType concept
• Use concepts instead of SFINAE¹²
• Use __VA_OPT__ instead of ##__VA_ARGS__
• Remove IndexT template parameter from BucketBase
• Replace InternalContractDataMap implementation with std::unordered_set heterogenous lookup

// Saving for POD types to binary
template <class T>
inline typename std::enable_if<std::is_arithmetic<T>::value, void>::type

This doesn't actually check for POD types (trivial + standard-layout), so the concept retains a std::is_arithmetic check.

Footnotes

1. There were some places that I left.
   src/test/Catch2.h:47: struct StringMaker<T, std::enable_if_t<xdr::xdr_traits<T>::valid>>
   because Catch2 still uses SFINAE
   Leaving XDRCereal.h/XDRCereal.cpp because I'm working on a separate PR to update it.
   Leaving FuzzerImpl.cpp alone since iirc Graydon was working on fuzzing. ↩

2. Note on the update to src/util/BufferedAsioCerealOutputArchive.h ↩

[Copilot]

Pull request overview

This PR modernizes the codebase to use C++20 features, replacing older C++ patterns with cleaner modern equivalents. The changes include migrating from SFINAE to concepts, adopting __VA_OPT__ for variadic macros, and leveraging C++20's heterogeneous lookup capabilities. Several third-party dependencies and the build system have been updated to support C++20.

Changes:

• Replaced SFINAE std::enable_if with C++20 concepts throughout the codebase
• Replaced BUCKET_TYPE_ASSERT macro with IsBucketType concept
• Simplified InternalContractDataMap implementation using heterogeneous lookup
• Updated logging macros to use __VA_OPT__ instead of ##__VA_ARGS__

Reviewed changes

Copilot reviewed 70 out of 70 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
configure.ac, README.md, INSTALL.md	Updated to require C++20
src/util/Logging.h	Replaced ##__VA_ARGS__ with __VA_OPT__ in logging macros
src/bucket/BucketUtils.h	Replaced BUCKET_TYPE_ASSERT macro with IsBucketType concept
src/bucket/BucketBase.h	Removed IndexT template parameter from BucketBase
src/ledger/InMemorySorobanState.h	Simplified to use std::unordered_set with heterogeneous lookup
src/util/XDROperators.h	Replaced operator< with operator<=>
src/herder/test/HerderTests.cpp	Fixed typo in function name and added test cases
lib/xdrpp, lib/spdlog, lib/fmt	Updated submodule commits

Comments suppressed due to low confidence (1)

src/util/Logging.cpp:1

• The change from fmt::localtime(time) to *std::localtime(&time) should be documented. The latter uses raw pointers and direct dereferencing which could be problematic if std::localtime returns nullptr (which can happen). Consider adding a comment explaining why this change was necessary or add a null check.

[bboston7]

I think this is close. Just a few small things I noticed.

[bboston7]

This is as close as I could get this comment to the real thing I'd like to comment on: Should isBucketMetaEntry and bucketEntryToLedgerEntryAndDurabilityType also use IsBucketType? Looks like they're the only two places left that use class BucketT.

[bboston7]

I found this pretty confusing on first glance. I suspect it will become more obvious as we get more used to C++20, but could you please add a comment explaining that C++20s subsumption rules mean that this function will be called when T is a SecretKey because C++20 always picks the most specific matching constraint? That is, there is no ambiguity even though the other toStrKey's constraint also technically matches. The same goes for toShortString.

[bboston7]

Another comment for an unrelated spot in the code: Line 339 has a comment that says "InternalContractDataMapEntry has an explicit copy constructor that deep-copies via clone(), so we can just use emplace," but I don't think that's true anymore. Is it OK that we're no longer making a deep copy there?

[bboston7]

Suggested change

	// This concept template makes is such that this function is only defined
	// This concept template makes it such that this function is only defined