[C++] Half-float builder should accept Float16

[pitrou]

Describe the enhancement requested

Currently, HalfFloatBuilder only allows passing uint16_t values. It should probably accept util::Float16 as a convenience.

Component(s)

C++

[pitrou]

cc @dinse

[dinse]

Working on this.

For AppendValues, would a loop or the following be preferable? cppreference rules for reinterpret_cast say "A pointer to an object of standard-layout class type can be reinterpret_cast to pointer to its first non-static non-bitfield data member. ... Note that strict aliasing rules still apply to the result of such cast." The code happens to work and my interpretation is it's allowed, but that isn't my area of expertise.

class HalfFloatBuilder : public NumericBuilder<HalfFloatType> {

using BaseClass = NumericBuilder<HalfFloatType>;
using BaseClass::AppendValues;

...

Status AppendValues(const Float16* values, int64_t length,
 const uint8_t* valid_bytes = NULLPTR) {
 return BaseClass::AppendValues(reinterpret_cast<const uint16_t*>(values), length, valid_bytes);
}

[pitrou]

We have this in util/float16.h, so I think the reinterpret_cast approach should work (and will definitely be faster):

static_assert(std::is_trivial_v<Float16>);
static_assert(sizeof(Float16) == sizeof(uint16_t));

That said, I think we can start with single-value Append if that's easier.

Also cc @bkietz for opinions.