Change Unit representaiton from {} to undefined

[JordanMartinez]

Description of the change

Fixes #266. Did we want to change the wording for the docs at all? Perhaps just say "don't return a value at all" instead?

---

Checklist:

• Added the change to the changelog's "Unreleased" section with a reference to this PR (e.g. "- Made a change (#0000)")
• Linked any existing issues or proposals that this pull request should close
• Updated or added relevant documentation
• Added a test for the contribution (if applicable)

[hdgarrood]

I think the docs probably ought to be changed, but that requires us to make a decision on the question I posed on the issue (in short: is the runtime representation of Unit part of the public API or not). In the meantime I think this can go in.

[paf31]

This means you won’t be able to tell if a field in a record with type unit field is present or not, surely?

[garyb]

This means you won’t be able to tell if a field in a record with type unit field is present or not, surely?

It's still possible to tell - hasOwnProperty and "field" in record both do the right thing, but it's a good point, we'll need to check the implementation is not doing !!record["field"] or equivalent things to check for the presence of keys.

• Compiler
• Record
• Object

[JordanMartinez]

@garyb Could you explain the compiler check and where specifically we'd need to look?

[garyb]

I've just had another look myself. Pretty sure the compiler-generated code is all good, the only compiler-generated code for records that I can think of is constructing records, and modifying fields in records. When modifying fields of open records it does use hasOwnProperty, and for closed records it assigns the unchanged properties with whatever value is in the source.

I also did a search for references to undefined, and as far as I could see we're just inserting or erasing undefineds directly from the AST, never making equality comparisons with them.

[JordanMartinez]

When I searched for unit throughout the codebase, I only came across unit, which is only used typeClassDictionaryDeclaration. That unit is a completely separate thing, right?

[garyb]

Oh, interesting. Yes and no... that unit value is the SourceType of {}. I think it's something that only exists to make the dictionary constructor code generation work correctly though... so actually, I guess yeah we can say that's entirely separate, but maybe we should name this something else. This is most likely from the early days before we erased "empty" arguments, and at some point a {} would actually have been passed in here.

[JordanMartinez]

Can I get an approval, so I can merge this? Seems like all issues have been addressed.

[JordanMartinez]

Thanks!