Should copy_loggable be enabled by default for all trivially copyable types?

[usefulcat]

In other words, should the default implementation of copy_loggable look like this:

template <typename T>
struct copy_loggable : std::bool_constant<std::is_trivially_copyable<T>::value>
{
};

I find that often when I need to mark a type as copy_loggable, it's already trivially copyable, and it seems like that ought to be the appropriate default for copy_loggable. But since I'm no language lawyer I doubt that I understand all the possible implications of this change.

[odygrd]

I think we can probably add it, at the moment it is only checking for trivial types https://github.com/odygrd/quill/blob/master/quill/include/quill/detail/misc/TypeTraitsCopyable.h#L314C38-L314C52

Do you have an example of a type that it happened ?

[usefulcat]

One example is a TimeDelta type, which represents the difference between two points in time (it's in some older code that predates std::chrono). It has quite a few different constructors, but no virtual methods and the only data member is an int64_t. It is trivially copyable (std::is_trivially_copyable_v == true).

I've just gotten started converting this codebase to use quill and fmtlib, but I have quite a few other such types as well. It's automated trading stuff so I also have things like Quote, Offer, Price, Order, all of which are trivially copyable POD types.

[odygrd]

I have done this in b34c828
Do you need a release in order to use it, or are you able to use master branch directly ?

[usefulcat]

Thanks! No release needed, I can get it from the master branch

[odygrd]

Thanks, let me know if you come across any other issues :)