boost::units::quantity should be trivial

[hlewin]

I think boost::units::quantity<> should be as trivial as possible.

Right now they are not ( std::is_trivial_v is false ) because of more-or-less superfluous implementations of constructors and assignment operators. The internally asserted layout compatibilities have no meaning for users of the class.

I do not really have a strong argument for making them trivial but have a strong gut feeling they should be (for users at least).

[correaa]

(Disclaimer: I am not a developer of Boost.Units, I am just a user, and I think there are strong arguments for this).

I agree with this.
I think it is a justified design, but history is not on our side.

For a while, I mistakenly thought that it did make sense for quantities to have a (non-trivial) zero initialization because zero values are, well, special for quantities/measurements.
With time, I realized that it is not a good reason, mainly from reading the book "Elements of Programming" and some related talks https://www.youtube.com/watch?v=HcSAIZafNZU "Partially-Formed Objects For Fun And Profit".

Many types are unfortunately stuck on being non-trivial when they shouldn't be, for example, std::pair of trivials or even std::complex.
https://en.cppreference.com/w/cpp/numeric/complex
Non-trivial std::complex, in particular, is a disaster for people doing high-performance numerics.

I don't know why this problem is so prevalent in old (even standard) libraries; I think the problem is that in C++98, it was not very easy to make types trivial (no = default;), so you had to write a lot of class specializations.

This an analogous request I made to have another complex type be trivial: NVIDIA/thrust#1079

And I am afraid what you bring up is exactly the same problem for std::quantity, which I also consider a numeric type.
The problem I observe is that it is almost impossible to change a type and make a trivial after the fact, considering backward compatibility.

As much as I would love std::quantity to be trivial (especially trivially constructor), I hate to be the bearer of bad news: the problem looking forward is that if some people already relied on zero initialization, this will be a breaking change.

Finally, as another datapoint, note that std::chrono::duration (a time quantity implementation), a model for modern C++ classes, is trivial: https://godbolt.org/z/Ej8rKEsG5 .

[hlewin]

There could be ifdefs for such things. There are no necessities. Just gut feelings (hope they ate well).

[NAThompson]

It indeed should be trivial-without it generic code using atomics does not work:

/opt/homebrew/opt/llvm/bin/../include/c++/v1/__atomic/cxx_atomic_impl.h:315:37: fatal error: _Atomic cannot be applied to type 'boost::units::quantity<boost::units::unit<boost::units::list<boost::units::dim<boost::units::length_base_dimension, boost::units::static_rational<2>>, boost::units::dimensionless_type>, boost::units::homogeneous_system<boost::units::list<boost::units::si::meter_base_unit, boost::units::list<boost::units::scaled_base_unit<boost::units::cgs::gram_base_unit, boost::units::scale<10, static_rational<3>>>, boost::units::list<boost::units::si::second_base_unit, boost::units::list<boost::units::si::ampere_base_unit, boost::units::list<boost::units::si::kelvin_base_unit, boost::units::list<boost::units::si::mole_base_unit, boost::units::list<boost::units::si::candela_base_unit, boost::units::list<boost::units::angle::radian_base_unit, boost::units::list<boost::units::angle::steradian_base_unit, boost::units::dimensionless_type>>>>>>>>>>>>' which is not trivially copyable
  315 |   _LIBCPP_DISABLE_EXTENSION_WARNING _Atomic(_Tp) __a_value;
      |                                     ^

[NAThompson]

@jhunold , @swatanabe : Any hope for getting this patchfile accepted?

make_trivial.patch

[hlewin]

If changing behaviour is not an option at all, I'd suggest introducing new namespace for such fixes.

[NAThompson]

@hlewin : What behavior is changed?

[correaa]

@NAThompson, as I said (in my convoluted comment), the current behavior initializes the value to zero. A proper trivial type doesn't initialize to zero; it "leaves" the value uninitialized.

If someone's code relied on that default initialization producing a zero value, it would be broken by making the type trivial.

It is unfortunate that quantity was not trivial from the start.

I had a similar dilemma concerning changing behavior.
I made similar changes in an internal version of B.Units.
For example, in most cases, I really wanted implicit conversion between compatible units.

For that, I have boost::units::implicit::quantity, which, in practice, I use as

void(implicit:: quantity<length>)

Analogously, I think what @hlewin is saying is that you can have: boost::units::trivial::quantity with your desired behavior.

[hlewin]

Analogously, I think what @hlewin is saying is that you can have: boost::units::trivial::quantity with your desired behavior.

The problem being that boost::quantity would need to inherit from boost::trivial::quantity - not the other way around.

[correaa]

Yes, I agree.

I was referring to the fact of using a new class only.

That is why trivializing a class after the fact is so difficult.

[hlewin]

Yes, I agree.

I was referring to the fact of using a new class only.

That is why trivializing a class after the fact is so difficult.

It is not: Declare the old behaviour deprecated and provide a define like BOOST_REMOVE_DEPRECATED to get rid of the constructors on a voluntary basis.

[correaa]

@hlewin I couldn't agree more that quantity should be trivial if T is trivial. I am just saying that it would be a breaking change. Moreover, I would agree with the breaking change if it were in my hands.

[hlewin]

@hlewin I couldn't agree more that quantity should be trivial if T is trivial. I am just saying that it would be a breaking change. Moreover, I would agree with the breaking change if it were in my hands.

Yeah, point is: Writing a patch to whatever spec is not really a problem. But I am not gonna waste that time if the general sentiment is to not accept it one way or the other. If there are arguments like "someone might have taken the sha256 checksum of that header so changing it might break a build" I am just outta here. I already have this patched in company local versions.

[correaa]

@hlewin , well, let's see what the developers say, if they say something. (I am not a developer or maintainer of Boost.Units).

I have my own patches add-ons, too.
These are the public ones https://gitlab.com/correaa/boost-unitx (including implicit:: quantity).

[NAThompson]

My own preference would be to make a breaking change, because well, std::atomic doesn't work with these types due to the triviality requirements, and therefore much of my multithreaded code simply doesn't compile with these types.

[hlewin]

Discussing the technicalities of a fix would only make sense if there would be consensus about that this is a problem with someone who can make such decisions. I don't see that. Zero-initialization of numerical values was a javaesque design decision back then and did not age well on the c++ side either. So... I don't hold my breath.
Have a nice day!