assemblyscript-temporal icon indicating copy to clipboard operation
assemblyscript-temporal copied to clipboard

Discussion about which type to use for different units

Open SebastianSpeitel opened this issue 3 years ago • 2 comments

Since the specification is defined for JS, it only talks about integers for most units and bigint for nanoseconds but since we are in a bit more strictly typed environment, we can be more thoughtful about our choice of integers.

Currently we are using i32 to represent JS integers and i64 for bigints, which is probably the closest we can get to the specification, except that biginits could be arbitrarily large and i64 can't.

Even if we continue to use this representation we should document or even enforce this via code somewhere.

Here are just my thoughts:

  • i32 is a bit much to represent a month (same for a few others like day)
    • use an enum
      • enums are just i32 in AS and aren't enforced to stay a possible value
      • an alternative is to use a namespace
    • use an u8
      • I already implemented this here: https://github.com/ColinEberhardt/assemblyscript-temporal/compare/master...SebastianSpeitel:i8-month
      • Only reduces memory if multiple smaller properties can be aligned together otherwise just leaves 3 bytes unused
      • Performance benefit would be minimal if even measurable, since wasm only knows i32 anyways
      • Requires a lot of casts, which could even make it slower
    • a lot of code uses months before normalization, after rounding or as a duration which would allow values outside of u8-range and force us to use a bigger type there anyways (different type for month and months?)
  • should duration units have the same type as their fixed versions?
  • Since Instant is just a wrapper around it's underlying nanoSeconds value could we just extend it?
  • Do i64 cause any problems at the border to js when converting to bigints?
  • A simple table in the docs covering which units use which type would be useful.
  • Maybe a type alias for each unit could be useful, even if they all alias the i32 type.

SebastianSpeitel avatar Mar 28 '21 17:03 SebastianSpeitel

Do i64 cause any problems at the border to js when converting to bigints?

some calculations for Instant will required i128 / u128. For example here and here

MaxGraey avatar Mar 28 '21 20:03 MaxGraey

The following two tests have been disabled, they require > 32 bit integers on the Duration type:

https://github.com/ColinEberhardt/assemblyscript-temporal/commit/438e0e196e111f413910e68e4236d6b194e59913

ColinEberhardt avatar Apr 26 '21 08:04 ColinEberhardt