-
Notifications
You must be signed in to change notification settings - Fork 396
Fix #5135: Move well-known names to a new object WellKnownNames. #5136
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
Conversation
…mes`. This breaks the circular dependency between the initialization of `object Types` and `object Names`.
This also serves as a test that there is no deadlock between `Names` and `Types` anymore. Without that fix, the addtion of `NamesTest` triggered a deadlock in virtually every run of `ir2_12/test`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments on the last commit.
IMO it can also be split off into another PR so we can merge the first two quicker. Up to you.
* circular dependency between the `PrimTypeWithRef`s and their `PrimRef`s. | ||
*/ | ||
|
||
def primRef: PrimRef = (charCode: @switch) match { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you considered val primRef = PrimeRef(this)
and then get the refs from the types for the aliases below?
case NullType => 'N' | ||
case NothingType => 'E' | ||
} | ||
val charCode: Char = tpe.charCode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def
?
case DoubleType => "double" | ||
case NullType => "null" | ||
case NothingType => "nothing" | ||
val displayName: String = (tpe.charCode: @switch) match { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of the match, have you considered adding displayName
as a private field to PrimTypeWithRef
and just forward here?
That would essentially make PrimRef
a "view" on a PrimType, which is maybe clearer overall?
e0d7551
to
53ecbe3
Compare
OK, I moved the 3rd commit to another branch https://github.com/sjrd/scala-js/tree/rearrange-init-sequence-of-primref . I'll turn it into a PR after this one is merged. It only contains the first two commits, now. |
And related changes.