Skip to content

Commit

Permalink
Merge bitcoin#29056: refactor: Print verbose serialize compiler error…
Browse files Browse the repository at this point in the history
… messages

fae5263 Allow std::byte C-style array serialization (MarcoFalke)
fa898e6 refactor: Print verbose serialize compiler error messages (MarcoFalke)

Pull request description:

  Currently, trying to serialize an object that can't be serialized will fail with a short error message. For example, the diff and the error message:

  ```diff
  diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp
  index d75eb499b4..773f49845b 100644
  --- a/src/test/serialize_tests.cpp
  +++ b/src/test/serialize_tests.cpp
  @@ -62,6 +62,8 @@ public:

   BOOST_AUTO_TEST_CASE(sizes)
   {
  +    int b[4];
  +    DataStream{} << b << Span{b};
       BOOST_CHECK_EQUAL(sizeof(unsigned char), GetSerializeSize((unsigned char)0));
       BOOST_CHECK_EQUAL(sizeof(int8_t), GetSerializeSize(int8_t(0)));
       BOOST_CHECK_EQUAL(sizeof(uint8_t), GetSerializeSize(uint8_t(0)));
  ```

  ```
  ./serialize.h:765:6: error: member reference base type 'const int[4]' is not a structure or union
    765 |     a.Serialize(os);
        |     ~^~~~~~~~~~
  ```
  ```
  ./serialize.h:277:109: error: no matching function for call to 'UCharCast'
    277 | template <typename Stream, typename B> void Serialize(Stream& s, Span<B> span) { (void)/* force byte-type */UCharCast(span.data()); s.write(AsBytes(span)); }
        |                                                                                                             ^~~~~~~~~
  ```

  This is fine. However, it would be more helpful for developers and more accurate by the compiler to explain why each function is not selected.

  Fix this by using C++20 concepts where appropriate.

ACKs for top commit:
  ajtowns:
    reACK fae5263
  achow101:
    ACK fae5263
  TheCharlatan:
    Re-ACK fae5263

Tree-SHA512: e03a684ccfcc5fbcad7f8a4899945a05989b555175fdcaebdb113aff46b52b4ee7b467192748edf99c5c348a620f8e52ab98bed3f3fca88280a64dbca458fe8a
  • Loading branch information
achow101 committed Dec 21, 2023
2 parents eefe4ba + fae5263 commit dca0f23
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 14 deletions.
2 changes: 2 additions & 0 deletions src/.clang-format
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,7 @@ SpacesInAngles: false
SpacesInContainerLiterals: true
SpacesInCStyleCastParentheses: false
SpacesInParentheses: false
BreakBeforeConceptDeclarations: Always
RequiresExpressionIndentation: OuterScope
Standard: c++20
UseTab: Never
29 changes: 16 additions & 13 deletions src/serialize.h
Original file line number Diff line number Diff line change
Expand Up @@ -271,10 +271,9 @@ template<typename Stream> inline void Serialize(Stream& s, int32_t a ) { ser_wri
template<typename Stream> inline void Serialize(Stream& s, uint32_t a) { ser_writedata32(s, a); }
template<typename Stream> inline void Serialize(Stream& s, int64_t a ) { ser_writedata64(s, a); }
template<typename Stream> inline void Serialize(Stream& s, uint64_t a) { ser_writedata64(s, a); }
template<typename Stream, int N> inline void Serialize(Stream& s, const char (&a)[N]) { s.write(MakeByteSpan(a)); }
template<typename Stream, int N> inline void Serialize(Stream& s, const unsigned char (&a)[N]) { s.write(MakeByteSpan(a)); }
template <typename Stream, typename B, std::size_t N> void Serialize(Stream& s, const std::array<B, N>& a) { (void)/* force byte-type */UCharCast(a.data()); s.write(MakeByteSpan(a)); }
template <typename Stream, typename B> void Serialize(Stream& s, Span<B> span) { (void)/* force byte-type */UCharCast(span.data()); s.write(AsBytes(span)); }
template <typename Stream, BasicByte B, int N> void Serialize(Stream& s, const B (&a)[N]) { s.write(MakeByteSpan(a)); }
template <typename Stream, BasicByte B, std::size_t N> void Serialize(Stream& s, const std::array<B, N>& a) { s.write(MakeByteSpan(a)); }
template <typename Stream, BasicByte B> void Serialize(Stream& s, Span<B> span) { s.write(AsBytes(span)); }

#ifndef CHAR_EQUALS_INT8
template <typename Stream> void Unserialize(Stream&, char) = delete; // char serialization forbidden. Use uint8_t or int8_t
Expand All @@ -288,10 +287,9 @@ template<typename Stream> inline void Unserialize(Stream& s, int32_t& a ) { a =
template<typename Stream> inline void Unserialize(Stream& s, uint32_t& a) { a = ser_readdata32(s); }
template<typename Stream> inline void Unserialize(Stream& s, int64_t& a ) { a = ser_readdata64(s); }
template<typename Stream> inline void Unserialize(Stream& s, uint64_t& a) { a = ser_readdata64(s); }
template<typename Stream, int N> inline void Unserialize(Stream& s, char (&a)[N]) { s.read(MakeWritableByteSpan(a)); }
template<typename Stream, int N> inline void Unserialize(Stream& s, unsigned char (&a)[N]) { s.read(MakeWritableByteSpan(a)); }
template <typename Stream, typename B, std::size_t N> void Unserialize(Stream& s, std::array<B, N>& a) { (void)/* force byte-type */UCharCast(a.data()); s.read(MakeWritableByteSpan(a)); }
template <typename Stream, typename B> void Unserialize(Stream& s, Span<B> span) { (void)/* force byte-type */UCharCast(span.data()); s.read(AsWritableBytes(span)); }
template <typename Stream, BasicByte B, int N> void Unserialize(Stream& s, B (&a)[N]) { s.read(MakeWritableByteSpan(a)); }
template <typename Stream, BasicByte B, std::size_t N> void Unserialize(Stream& s, std::array<B, N>& a) { s.read(MakeWritableByteSpan(a)); }
template <typename Stream, BasicByte B> void Unserialize(Stream& s, Span<B> span) { s.read(AsWritableBytes(span)); }

template <typename Stream> inline void Serialize(Stream& s, bool a) { uint8_t f = a; ser_writedata8(s, f); }
template <typename Stream> inline void Unserialize(Stream& s, bool& a) { uint8_t f = ser_readdata8(s); a = f; }
Expand Down Expand Up @@ -755,18 +753,23 @@ template<typename Stream, typename T> void Serialize(Stream& os, const std::uniq
template<typename Stream, typename T> void Unserialize(Stream& os, std::unique_ptr<const T>& p);



/**
* If none of the specialized versions above matched, default to calling member function.
*/
template<typename Stream, typename T>
inline void Serialize(Stream& os, const T& a)
template <class T, class Stream>
concept Serializable = requires(T a, Stream s) { a.Serialize(s); };
template <typename Stream, typename T>
requires Serializable<T, Stream>
void Serialize(Stream& os, const T& a)
{
a.Serialize(os);
}

template<typename Stream, typename T>
inline void Unserialize(Stream& is, T&& a)
template <class T, class Stream>
concept Unserializable = requires(T a, Stream s) { a.Unserialize(s); };
template <typename Stream, typename T>
requires Unserializable<T, Stream>
void Unserialize(Stream& is, T&& a)
{
a.Unserialize(is);
}
Expand Down
6 changes: 5 additions & 1 deletion src/span.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <algorithm>
#include <cassert>
#include <cstddef>
#include <span>
#include <type_traits>

#ifdef DEBUG
Expand Down Expand Up @@ -283,13 +284,16 @@ Span<std::byte> MakeWritableByteSpan(V&& v) noexcept
return AsWritableBytes(Span{std::forward<V>(v)});
}

// Helper functions to safely cast to unsigned char pointers.
// Helper functions to safely cast basic byte pointers to unsigned char pointers.
inline unsigned char* UCharCast(char* c) { return reinterpret_cast<unsigned char*>(c); }
inline unsigned char* UCharCast(unsigned char* c) { return c; }
inline unsigned char* UCharCast(std::byte* c) { return reinterpret_cast<unsigned char*>(c); }
inline const unsigned char* UCharCast(const char* c) { return reinterpret_cast<const unsigned char*>(c); }
inline const unsigned char* UCharCast(const unsigned char* c) { return c; }
inline const unsigned char* UCharCast(const std::byte* c) { return reinterpret_cast<const unsigned char*>(c); }
// Helper concept for the basic byte types.
template <typename B>
concept BasicByte = requires { UCharCast(std::span<B>{}.data()); };

// Helper function to safely convert a Span to a Span<[const] unsigned char>.
template <typename T> constexpr auto UCharSpanCast(Span<T> s) -> Span<typename std::remove_pointer<decltype(UCharCast(s.data()))>::type> { return {UCharCast(s.data()), s.size()}; }
Expand Down

0 comments on commit dca0f23

Please sign in to comment.