Basic values and tests

[Kukovec]

Partially addresses #23 (missing compound values, e.g. Map/Vec)

[konnov]

Can we avoid using abstract classes and classes here? There is no need for them. An idiomatic way in TypeScript is to rather use discriminated unions: { kind: 'int32', val: bigint } | { kind: 'bool', val: boolean }.

The reason is that we would run into multiple issues with classes such as instanceof, equality, hashing, etc.

[konnov]

Also, it's quite easy to construct values with discriminated unions like { kind: 'bool', val: true }, instead of instantiating classes.

[Kukovec]

Can we avoid using abstract classes and classes here? There is no need for them. An idiomatic way in TypeScript is to rather use discriminated unions: { kind: 'int32', val: bigint } | { kind: 'bool', val: boolean }.

The reason is that we would run into multiple issues with classes such as instanceof, equality, hashing, etc.

And how do you enforce field validity in those discriminated unions? The class constructors make sure u32 values are actually u32, I don't see a way of doing that with your suggestion.

[konnov]

Can we avoid using abstract classes and classes here? There is no need for them. An idiomatic way in TypeScript is to rather use discriminated unions: { kind: 'int32', val: bigint } | { kind: 'bool', val: boolean }.
The reason is that we would run into multiple issues with classes such as instanceof, equality, hashing, etc.

And how do you enforce field validity in those discriminated unions? The class constructors make sure u32 values are actually u32, I don't see a way of doing that with your suggestion.

I am actually not sure about why you want to have field validation there. You definitely do not want validation in many cases, e.g., when copying values. This is typically done somewhere in the parser/reader. If you need validation, simply introduce a validation function that does a switch over different kinds. The intermediate representation is just data.

[Kukovec]

Can we avoid using abstract classes and classes here? There is no need for them. An idiomatic way in TypeScript is to rather use discriminated unions: { kind: 'int32', val: bigint } | { kind: 'bool', val: boolean }.
The reason is that we would run into multiple issues with classes such as instanceof, equality, hashing, etc.

And how do you enforce field validity in those discriminated unions? The class constructors make sure u32 values are actually u32, I don't see a way of doing that with your suggestion.

I am actually not sure about why you want to have field validation there. You definitely do not want validation in many cases, e.g., when copying values. This is typically done somewhere in the parser/reader. If you need validation, simply introduce a validation function that does a switch over different kinds. The intermediate representation is just data.

Can you give me an example of such copying, where you would explicitly not want the validity check, because I genuinely don't understand what use case you have in mind.

[konnov]

[...]

Can you give me an example of such copying, where you would explicitly not want the validity check, because I genuinely don't understand what use case you have in mind.

I don't want to get into a discussion between classes and discriminated unions in TypeScript. What I can say is that there are very few cases, where I have seen value in using classes (I mean, TypeScript classes) in TypeScript. Most of the time, plain objects and discriminated unions do the job just fine. Here are just a few things that make TS classes hard to use: useless equality, useless toString() by default, no good support for instanceof (you still have to introduce the kind field).

In your PR, we need something similar to sealed classes in Scala. Discriminated unions provide you with exactly that. You don't want to inherit from values, nor you need other features of classes.

[konnov]

I like it really much better! Left a few stylistic comments.

[konnov]

Why would not you simply enumerate all cases right here? You still have to define types for IntValue_u32 below.

For instance, this is just immediate and does not require any scrolling:

export type IntValue =
    { val: bigint, type: "u32" }
  | { val: bigint, type: "i32" }
  | { val: bigint, type: "u64" }
  ...

Alternatively, we could do:

export type IntValue =
  { type: "uint", bits: 32 | 64 | 128, val: bigint }
  { type: "int", bits: 32 | 64 | 128, val: bigint }

[konnov]

what is the value of these definitions?

[konnov]

if we already know that we have an unsigned u32 here, why don't we just use number? Most likely, you would have to use the array length in loops, which would make it much easier if we are using number.

[konnov]

Looks good! Let's merge it. I think you would still run into issues when looping over bigints, but we can deal with that later.