search

nholthaus / units

a compile-time, header-only, dimensional analysis and unit conversion library built on c++14 with no dependencies.
http://nholthaus.github.io/units/
MIT License
947 stars 135 forks source link

Add common type of time units and `std::chrono::duration` #217

Closed JohelEGP closed 3 years ago

JohelEGP commented 5 years ago

Working towards correct std::common_type specializations. First, put everything in one place.

JohelEGP commented 5 years ago

This is ready.

It'd be great to add some tests for the common type of units that include non linear units, like temperature units, decibel scaled units, and that of constant::pi. There are some tests for their types, but not their values. That's to confirm that using the gcd of their ratios is correct.

I don't think it's possible to make the common type of dimensionless units and arithmetic types work correctly. There's the requirement: "Moreover, common_­type_­t<T1, T2> shall denote the same type, if any, as does common_­type_­t<T2, T1>." But you can't add partial specializations for that without introducing ambiguity. I wonder how the SG6 proposals handle that.

JohelEGP commented 5 years ago

This is ready.

Actually, wait. This change and previous specializations also failed to meet this requirement: "a program may specialize common_­type<T1, T2> for types T1 and T2 such that is_­same_­v<T1, decay_­t<T1>> and is_­same_­v<T2, decay_­t<T2>> are each true." Partial specializations that use a template parameter as an argument in the argument-list of the specialization, like this StrongUnit and this T, could accept types that weren't the same if decayed. That would mean that for each strong unit of the same dimension, say millimeters through kilometers, you'd have to "manually" define partial specializations in pairs of all their combinations. And this has to expand to non-meters length units, like feet. This is unreasonable, and explodes the amount of symbols in the library even more. std::common_type has a really unfriendly interface for specializing in cases such as this.

What do you think about solving this by reverting making the units strong types?

JohelEGP commented 5 years ago

Changed the scope of the PR to adding these std::common_type specializations between time units and std::chrono::duration specializations.

nholthaus commented 3 years ago

@johelegp I'm a bit confused by the comment timeline between this and #219, but I think that we still want to pull this PR even if we revert strong units?

JohelEGP commented 3 years ago

That's right.

nholthaus commented 3 years ago

I made an attempt to resolve the conflicts but I wasn't confident in the results. Would you mind taking a stab at it?