Skip to content

Commit

Permalink
Make bit_cast compatible with old C++11 compilers
Browse files Browse the repository at this point in the history
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 `<type_traits>` header, but the
provided `<type_traits>` 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<From>::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<To>::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`).
  • Loading branch information
generalmimon committed Jul 9, 2024
1 parent c8b875b commit 1fbe3e8
Showing 1 changed file with 17 additions and 7 deletions.
24 changes: 17 additions & 7 deletions kaitai/kaitaistream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,23 +61,33 @@
#include <vector> // std::vector

#ifdef KAITAI_STREAM_H_CPP11_SUPPORT
#include <type_traits> // std::enable_if, std::is_trivially_copyable, std::is_trivially_constructible
#include <type_traits> // 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<class To, class From>
typename std::enable_if<
sizeof(To) == sizeof(From) &&
std::is_trivially_copyable<From>::value &&
std::is_trivially_copyable<To>::value,
std::is_trivial<From>::value &&
std::is_trivial<To>::value,
To
>::type
// constexpr support needs compiler magic
static bit_cast(const From &src) noexcept
{
static_assert(std::is_trivially_constructible<To>::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<To>::value,
// "This implementation additionally requires "
// "destination type to be trivially default constructible");

To dst;
std::memcpy(&dst, &src, sizeof(To));
Expand Down

0 comments on commit 1fbe3e8

Please sign in to comment.