Rename NoType to VoidType and print it as "void". #5061
Conversation
| case object VoidType extends PrimTypeWithRef | ||
|
|
||
| @deprecated("Use VoidType instead", since = "1.18.0") | ||
| lazy val NoType: VoidType.type = VoidType |
There was a problem hiding this comment.
NoType is deprecated but now it refers VoidType but it seems that the tests are changed to use VoidType. Do the tests then still have coverage for NoType? Seems it does but not sure and thought I'd ask.
There was a problem hiding this comment.
No they don't have coverage for NoType but this isn't a big deal. It's mostly to help migration of whoever may directly use the IR library.
There was a problem hiding this comment.
I was wondering that too, versus just changing the code since it seemed "internal". I follow to try and learn since this project is very well run :-)
There was a problem hiding this comment.
Typo in the last commit message:
but for historical,
NoTypekept its old name
but for historical reasons, NoType...
Regarding the PR title: I'm not a fan that the PR title suggests this is a merely a rename / printer change, but there is (quite deeply hidden) a rather fundamental change of how we type record types.
To someone looking through a changelog / commit history, I think it would be quite difficult to identify that this change sequence has changed subtyping relationships.
(then again, records are internal, and any attempt to actually return a record type somewhere to JS would likely fail).
| * Used by the optimizer to inline classes as records with multiple fields. | ||
| * They are desugared as several local variables by JSDesugaring. | ||
| * Record types cannot cross method boundaries, so they cannot appear as | ||
| * the type of fields or parameters, nor as result types of methods. | ||
| * The compiler itself never generates record types. | ||
| * | ||
| * Record types currently do not feature any form of subtyping. For R1 to be | ||
| * a subtype of R2, it must have the same fiels, in the same order, with |
There was a problem hiding this comment.
| * a subtype of R2, it must have the same fiels, in the same order, with | |
| * a subtype of R2, it must have the same fields, in the same order, with |
Right, good point. I extracted that functional change in a separate PR #5065. |
The concept of "void" will be reserved for "statement types", not for expressions with the `undefined` value.
Also clarify that it is the real top type of our type system. We have been using `void` in the "theoretical" type system since Scala.js 1.0.0, but for historical reasons, `NoType` kept its old name. We now officially rename it to `VoidType`, and print it as `void` instead of `<notype>`, giving it a true `Type` status.
|
Rebased and fixed the commit message typo. |
And port the changes that were made to the compiler backend. Most nobably, the changes from the following upstream PRs: * Introduce non-nullable reference types in the IR. scala-js/scala-js#5018 * Opt: Remove useless *Ident indirections in the IR model. scala-js/scala-js#5092 * Merge the IR node This into VarRef, with a magic LocalName. scala-js/scala-js#5090 * Merge several IR nodes into UnaryOp. scala-js/scala-js#5088 * Restrict usages of StoreModule even further. scala-js/scala-js#5059 * Allow Return arg to be a void if the target Labeled is a void. scala-js/scala-js#5074 * Rename NoType to VoidType and print it as "void". scala-js/scala-js#5061
And port the changes that were made to the compiler backend. Most notably, the changes from the following upstream PRs: * Introduce non-nullable reference types in the IR. scala-js/scala-js#5018 * Opt: Remove useless *Ident indirections in the IR model. scala-js/scala-js#5092 * Merge the IR node This into VarRef, with a magic LocalName. scala-js/scala-js#5090 * Merge several IR nodes into UnaryOp. scala-js/scala-js#5088 * Restrict usages of StoreModule even further. scala-js/scala-js#5059 * Allow Return arg to be a void if the target Labeled is a void. scala-js/scala-js#5074 * Rename NoType to VoidType and print it as "void". scala-js/scala-js#5061
And port the changes that were made to the compiler backend. Most notably, the changes from the following upstream PRs: * scala-js/scala-js#5018 * scala-js/scala-js#5092 * scala-js/scala-js#5090 * scala-js/scala-js#5088 * scala-js/scala-js#5059 * scala-js/scala-js#5074 * scala-js/scala-js#5061
We have been using
voidin the "theoretical" type system since Scala.js 1.0.0, but for historical,NoTypekept its old name.We now officially rename it to
VoidType, and print it asvoidinstead of<notype>, giving it a trueTypestatus.I've been wanting to do this for a while now. I finally took it up. 😅