From 1fbe3e8a3a26193107b24ce359b4147992018ab1 Mon Sep 17 00:00:00 2001 From: Petr Pucil Date: Mon, 8 Jul 2024 00:55:36 +0200 Subject: [PATCH] Make `bit_cast` compatible with old C++11 compilers The current `bit_cast` implementation fails to compile on several old compilers such as `x86-64 gcc 4.9.4`, `x86-64 clang 3.6` or `x86-64 icc 13.0.1` (checked at https://godbolt.org/). All these compilers accept the `-std=c++11` flag and have the `` header, but the provided `` doesn't have `std::is_trivially_copyable` that we're trying to use, so they all report this error: ``` error: no member named 'is_trivially_copyable' in namespace 'std' std::is_trivially_copyable::value && ~~~~~^ ``` To fix this, we replace `std::is_trivially_copyable` with `std::is_trivial`, which seems to be supported since the very first compiler versions with C++11 support (it works in `x86-64 gcc 4.7.1`, `x86-64 clang 3.0.0` and `x86-64 icc 13.0.1`, which are the oldest versions available at https://godbolt.org/ that support the `-std=c++11` command-line option). "Trivial" types are apparently a subset of "trivially copyable" types - compare https://en.cppreference.com/w/cpp/named_req/TrivialType with https://en.cppreference.com/w/cpp/named_req/TriviallyCopyable and see https://en.cppreference.com/w/cpp/language/classes#Trivial_class: > #### Trivial class > > A *trivial class* is a class that > > - is trivially copyable, and > - has one or more [eligible default > constructors](https://en.cppreference.com/w/cpp/language/default_constructor#Eligible_default_constructor) > such that each is > [trivial](https://en.cppreference.com/w/cpp/language/default_constructor#Trivial_default_constructor). Therefore, `std::is_trivial` should guarantee `std::is_trivially_copyable`. We're probably making `bit_cast` a bit stricter than it needs to be, but that's perfectly fine for our purposes (in fact, we're only using scalar types at the moment). Unfortunately, as [this blog post](https://quuxplusone.github.io/blog/2024/04/02/trivial-but-not-default-constructible/) points out, just because a type is trivial doesn't mean it's also trivially default constructible. Our implementation needs the `To` type to be trivially default constructible because of the `To dst;` statement, so we'd like to check `std::is_trivially_default_constructible::value`. But we can't, because `std::is_trivially_default_constructible` is just as poorly supported (by old compilers) as `std::is_trivially_copyable`, which we replaced with `std::is_trivial` to fix the compatibility issues. So we omit this check and hope for the best. Technically, we could also replace `To dst;` with some other way to allocate (properly aligned) memory for the destination type that does not call the default constructor (and then we wouldn't have to check `std::is_trivially_default_constructible`). For example, [using `std::aligned_storage`](https://github.com/jfbastien/bit_cast/blob/c288208c1a68a329a5839f007200ab2ee2b65073/bit_cast.h#L57) or `union` (see https://stackoverflow.com/q/52338255/12940655 or https://akrzemi1.wordpress.com/2012/12/13/constexpr-unions/). However, we don't really need to support types that don't have a (trivial) default constructor, and trying to do so adds complexity and requires additional considerations (e.g. `std::aligned_storage` is deprecated in C++23, there seem to be [various restrictions](https://en.cppreference.com/w/cpp/language/union#Syntax) on types that can be stored in a `union`). --- kaitai/kaitaistream.cpp | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/kaitai/kaitaistream.cpp b/kaitai/kaitaistream.cpp index 940cc37..bc07545 100644 --- a/kaitai/kaitaistream.cpp +++ b/kaitai/kaitaistream.cpp @@ -61,23 +61,33 @@ #include // std::vector #ifdef KAITAI_STREAM_H_CPP11_SUPPORT -#include // std::enable_if, std::is_trivially_copyable, std::is_trivially_constructible +#include // std::enable_if, std::is_trivial // Taken from https://en.cppreference.com/w/cpp/numeric/bit_cast#Possible_implementation -// (only adjusted for C++11 compatibility) +// (for compatibility with early C++11 compilers like `x86-64 gcc 4.9.4`, `x86-64 clang 3.6` or +// `x86-64 icc 13.0.1`, `std::is_trivially_copyable` was replaced with `std::is_trivial` and the +// `std::is_trivially_default_constructible` assertion was omitted) template typename std::enable_if< sizeof(To) == sizeof(From) && - std::is_trivially_copyable::value && - std::is_trivially_copyable::value, + std::is_trivial::value && + std::is_trivial::value, To >::type // constexpr support needs compiler magic static bit_cast(const From &src) noexcept { - static_assert(std::is_trivially_constructible::value, - "This implementation additionally requires " - "destination type to be trivially constructible"); + // // NOTE: because of `To dst;`, we need the `To` type to be trivially default constructible, + // // which is not true for all trivial types: + // // https://quuxplusone.github.io/blog/2024/04/02/trivial-but-not-default-constructible/ + // // + // // However, we don't check this requirement (and just assume it's met), because + // // `std::is_trivially_default_constructible` is not supported by some (very) old compilers + // // with incomplete C++11 support (`x86-64 gcc 4.9.4`, `x86-64 clang 3.6` or + // // `x86-64 icc 13.0.1` at https://godbolt.org/). + // static_assert(std::is_trivially_default_constructible::value, + // "This implementation additionally requires " + // "destination type to be trivially default constructible"); To dst; std::memcpy(&dst, &src, sizeof(To));