Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SI prefixes #11

Closed
sharkdp opened this issue Sep 8, 2022 · 3 comments
Closed

SI prefixes #11

sharkdp opened this issue Sep 8, 2022 · 3 comments

Comments

@sharkdp
Copy link
Owner

sharkdp commented Sep 8, 2022

This needs to be checked rather sooner than later. Does our "user-defined units" concepts fare well with SI prefixes that can come directly before the identifier? Or do we only support those prefixes on specified units (we probably should)? Or only on a small set of builtin units (that would be a bit sad)?

kilometer => prefix(kilo) · identifier(meter)
km => prefix(kilo) · identifier(meter)

@sharkdp sharkdp added this to the MVP milestone Sep 11, 2022
@sharkdp sharkdp removed this from the MVP milestone Sep 20, 2022
@sharkdp
Copy link
Owner Author

sharkdp commented Sep 28, 2022

Implementation plan:

  • Write a special parser that works on identifiers. That could be a separate struct inside the existing parser module.
  • Crucially, this part of the parser needs access to the existing set of units.
  • This parser would take strings like km, kilometer, mi, cd, or just kilo (to support kilo meter) as input, and then
    • check if the full identifier corresponds to a known unit (e.g. mi for mile or cd for candela). If that's the case, we simply emit an Expression::Identifier(…) AST element.
    • if the identifier is not a known unit, we try to parse a SI or binary prefix. We first try the long versions (kilo, kibi, milli, etc.). If that fails, we try the short versions (k, Ki, m …). The remaining part of the identifier is then treated as the unit identifier. If that unit exists in the dictionary, and if that unit is a 'normal' unit, we emit an
      Expression::BinaryOperator(
        BinaryOperator::Mul,
        Expression::Prefix(),
        Expression::Identifier(remainder)
      )
    • If it's not a normal unit (one which does not take an SI prefix), or if the unit name does not exist, we emit a simple Expression::Identifier(…) because that could still be a variable name.

In addition, if the full identifier is a long name of a prefix, like kilo, we could emit Expression::Prefix(Prefix::Decimal, 3).

Let's check if this handles known problematic cases.

  • Let's say we have mi/mile and min as known units. Each of them could be supported because the first check simply looks up the name in the dictionary.
  • cd could be parsed as candela (instead of centi day) because that would be looked up first.
  • ft could be parsed as foot (instead of femto tonne).

Potential problems:

  • This turns our syntax into a context-dependent language ☹️. ft would either be a femto tonne. Or, if Imperial units are also defined.. it would be ft=foot. However, the situation is not worse than with global variables anywhere else. Just a bit more unexpected maybe. We could try to alleviate this by printing a warning/error: "ft is ambiguous: foot or femto tonne. Please use the long form. In order not to make this extremely annoying for everyone using ft=foot or cd=candela, we could somehow forbid usage of femto tonne and centi days by explicitly only allowing positive prefixes for tonne (kiloton, megaton, …) and disallowing them for day completely?
  • Let's say someone defines a custom unit named ns. Then, nano seconds would not be available anymore as a unit.

@sharkdp
Copy link
Owner Author

sharkdp commented Jan 27, 2023

Note: we can not delay this to compilation time because the type checker needs that information. it needs to know that kilometer is of type Length.

@sharkdp
Copy link
Owner Author

sharkdp commented Feb 13, 2023

This has been solved in a different way. We introduced a new stage to the compiler ("Prefix Transformer").

@sharkdp sharkdp closed this as completed Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant