v3.x: abbreviation & name are broken when using integral underlying types

[pattop]

units::unit<meters, int> c;
EXPECT_STREQ("m", units::abbreviation(c));
EXPECT_STREQ("meter", units::name(c));
EXPECT_STREQ("m", c.abbreviation());
EXPECT_STREQ("meter", c.name());

units.h:109:52: warning: inline function ‘constexpr const char* units::abbreviation(const T&) [with T = units::unit<units::unit_conversion<std::ratio<1>, units::dimension_t<units::dim<units::dimension::length_tag, std::ratio<1, 1> > > >, int>]’ used but never defined

It would also be nice to support querying for name/abbreviation without requiring an object, maybe something like:

EXPECT_STREQ("ft", units::abbreviation<foot_t>());
EXPECT_STREQ("foot", units::name<foot_t>());

Another minor thing, in the unit tests the nameAndAbbreviation test case is ifdef'd out by UNIT_LIB_DISABLE_IOSTREAM. That doesn't look quite right.

[JohelEGP]

Ah, that's because they're defined as a template and specialized for the defined units. I guess a more correct approach now that there's support for non-double units is to make them overloads.

[pattop]

Makes sense.

By the way, nice job on the integral type support. It's going to be really useful.

Off topic, but have you considered whether it would be possible/make sense to use something like https://github.com/johnmcfarlane/cnl as the underlying type?

[JohelEGP]

Thank you.

Strictly, the library only support arithmetic types. However, it's possible to relax this. I don't know if @nholthaus has plans for that.

I see the link is related to the Numbers TS. The answer is in whether it makes sense to use them as an underlying type of std::chrono::duration, which only loosely requires it to be arithmetic-like. I'd say your YMMV depending on the assumptions your implementations make of what's arithmetic-like. I guess that'll improve in the future when people start questioning whether the Numeric TS's types can be used with std::chrono::duration.

[JohelEGP]

I haven't been successful in finding a way to make units::name and units::abbreviation work with non-double types. Guidelines suggest to not use function templates as customization points trough specialization. As we're supporting program-defined units, this customization point should be redesigned accordingly.

[nholthaus]

@pattop I definitely want to be able to use John's fixed-point types as underlying types (and he really wants me to as well). I haven't done much work on it yet though, and I'm not sure if the cnl types are trivial or not, which is a requirement for units.

[nholthaus]

@johelegp I'm planning to split the unit definitions into different headers to reduce compile-times. I'm not really sure what the best way to deal with the name clashes from different underlying types is.

I guess the options I see are:

1. Some kind of Hungarian notation (hate this, but at least it's practical)
2. Namespace them differently, e.g. units::int::... (probably a very bad idea which would mess up the ADL)
3. Have different headers for different underlying types, and don't even attempt to resolve the name clashes (not user friendly)
4. Rearrange the templates to expose the underlying type to the user. (really don't like this b/c I think having the templates in the interface is unclean and unnecessarily complex for non-expert users)

... and a few even worse ideas. None of these approaches meets my quality threshold. Can we come up with some other ideas?

[JohelEGP]

I guess that by name clashing you mean having an accessible name for units with an int underlying type, like there's meter_t (to be meters, right?) with a double underlying type.

Regarding (2), it shouldn't affect ADL because they're aliases to units::unit. That's why the math functions work with ADL. I don't like the idea of (3), having different headers for different underlying types. Anyways, the library would only directly support double and probably int, right? The user would have to be explicit if they wanted anything else.

I prefer meters<>/meters<int> over meters/imeters, which are suggested by (1) and (4). For both units and std::chrono, the type of a unit that doesn't use the convenient type aliases are a mouthful. The units one are even more so, as it is more general, so I'd welcome imeters or meters<int> with open arms.

[pattop]

@nholthaus great news, the combination of units and John's fixed point types would be incredible.

[pattop]

I personally prefer metres<uint8_t>. To me it's the least surprising compared to other libraries.

I disagree that exposing the underlying types to end users is overly complex. This is C++, you're taught about vector<int> in lesson 1 or 2.

[Morwenn]

Using a template parameter that defaults to double actually allows to write almost unchanged code in C++17 thanks to implicit deduction guides. The following could work:

meters tmp1;  // meters<double>
meters<int> tmp2;  // meters<int>

You should of course be careful and extensively test construction, but that would be the most interesting solution IMO.

[JohelEGP]

Unfortunately, all units are aliases, which don't participate in CTAD. There's work going in the area, but I'll be at least until C++20.

[Morwenn]

Ouch, I forgot about that limitation, my bad.

[johnmcfarlane]

I've never thought to check but it looks like CNL's types are trivial. They're certainly literal types which is pretty important for using them in constant expressions. Of course, if you specialize them with something other than the default (int) then that can change.

Minor nitpick: I'd also argue that they are arithmetic types; they're just not fundamental arithmetic types which is what std::is_arithmetic really tests for. (example usage, handy reference)

[nholthaus]

@johnmcfarlane totally agree with you on that. I Wish libraries like ours could create new fundamental arithmetic types (or something that was equivalently treated), but I'm guessing that would fall under the extreme-expert complexity that the committee is trying to minimize in the new standards.

[johnmcfarlane]

It's not the easiest thing to do but there's no theoretical barrier to making first class number types. It's just that the standard isn't prepared for people trying yet. That things like is_integral only apply to fundamental types just means that we need better traits. P0437 goes a long way to fixing that but progress is slow because other things are higher priority in the committee right now. Once we have metaclasses, writing new number types suddenly becomes way easier. In the meantime, it's advantageous to make types play well together. Aside from anything else, you get a much more robust API.

[nholthaus]

@johelegp getting this thread back on track, yes, units are all aliases, but they don't really have to be. If we made the units structs that were inherited from the unit_t type would we be able to use CTAD? This is probably a bigger refactor than it sounds but I wouldn't mind if it got us that sweet syntax.

[JohelEGP]

Yes!

[nholthaus]

OK, it's officially on the list. I have a feeling it will make a lot of the error messages easier to read too.

[Morwenn]

Yep, struct being the closest thing to a strong typedef you get, the error messages shouldn't expand the base class template as long as they don't need to :)