Unit parser

[SolidWallOfCode]

This brings the unit parser logic into ATS. It provides generic support for parsing strings that have embedded units. Updating the set of units requires only changing the initialization data, making it very easy to add or remove additional units. This commit adds a concrete instantiation that supports time string parsing, so strings such as "1 day 12 hours 30 minutes" is correctly parsed. A primary goal is to stop using default / implied units for time values in configuration, but instead always require units while providing an easy way to specify convenient units.

See this fle for examples of use.

Related to #9477

Usage would be something like

auto && [ value, errata ] = ts::time_parser(value_string);
if (! errata.is_ok()) {
  std::string text;
  Warning(bwprint(text, "Failed to parse duration - {}", errata)).c_str());
  return TS_ERROR;
}
timeout = std::chrono::duration_cast<std::chrono::milliseconds>(value);

[maskit]

Having this parser is fine. It'd make sense for file/memory size, but I'm skeptical to use it for amount of time such as timeout.

abc.max_buf_size: 10M -- Makes sense, it might hit internal upper limit but I don't think there'd be confusion.
xyz.timeout: 100ms -- Even if there's no upper nor lower limit, It's not obvious whether milliseconds makes sense.

We currently use second for settings that receive amount of time in most of cases. And if a setting expects millisecond, the setting name has _ms suffix. It's obvious that the percision is millisecond (if not, the name should not have _ms and just use second like others).

It's not only about percision. The suffix roughly suggests reasonable amount of time. Setting 100000 to a setting that has _ms suffix looks odd.

Of course we could describe things on the documentation and say RTFM, but I don't think providing super detailed 1000 pages of documentation is user friendly.

[cmcfarlen]

Just curious, is all of this constexpr resolved to data at compile time or is there static initializer setup here?

[SolidWallOfCode]

No, there's still computation that occurs to initialize the parser data structure. I'm not sure what you mean by "static initializer" other than the parser being a static data structure which is constructed at process start time. That is,

UnitParser const time_parser{ ... };

which declares a static instance of UnitParser.

[cmcfarlen]

Yeah, I mean the static setup at process start time. It's not called "static initializing"?

I'd rather see time_parser be a function and just return a UnitParser and if a program wants one call. Then the programmer can decide its lifetime.

[SolidWallOfCode]

Why? It seems more performant to construct a single instance at process start and use that everywhere. Then there is no runtime construction / destruction cost. What's the benefit of multiple instances?

[brbzull0]

Not having units specified could end up in being a real mess, I like this.

[SolidWallOfCode]

It's certainly caused many problems in the past. One of my goals for the YAML conversion was to require explicit units for all time periods to avoid any possible confusion, and "random" numbers like "86400" instead of "1 day".

[maskit]

What were the problems?

[SolidWallOfCode]

People used wildly inappropriate values because they didn't realize the implied units. It's a bit less bad because one place this came up was where the default unit was minutes, not seconds. There was another case where the user assumed ms but it was really seconds.

[maskit]

Sounds like this fixes a problem, but those problems support my concern. People don't read the documentation. And, more characters, more typos.
I can imagine conversations like:

A: Hey, I set xyz to 12h but it doesn't work!
B: Oh, it's rounded to day. You could set it to 24h but less than 24h wouldn't work as you expect.

A: Hey, I set the timeout to 100ms but it doesn't work!
B: Can you share your records.yml?
B: There's a typo. You set it to 100 nanoseconds

A: Hey, I set another timeout to 800ms, and it doesn't work. I'm sure I set it to 800 millisecond this time.
B: Oh, unlike the other timeout, this one is a bit special and it's not that precise. I'd use second.

Nothing is going to be perfect. I wonder which is better.

[bneradt]

Makes sense. Should be straightforward to add some unit tests?

[SolidWallOfCode]

Based on comments, I moved the actual unit parser instance to a nested namespace and wrapped it with a function to do the conversion to std::chrono units so that doesn't have to be repeated with the risk of error. Comments were added to the function to make use clearer.

[brbzull0]

Looks good Alan! Thanks.