-
Notifications
You must be signed in to change notification settings - Fork 23
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
Case objects in new runtime removing restrictions #209
Comments
The workaround we usually use in the compiler and elsewhere is partial construction like so var x = MyCaseObj(sStr: true)
x.s = "ola"
Which is IME good enough for serialization and the like. Are you aware of this solution and if so, why doesn't it work for your cases? |
As for the implementation... If we decide that we need the feature, avoid type system extensions like the plague and go with any solution that avoids it. The type system is much more complex than the AST or the "symbol kind" structure. |
Hi Araq, I am aware of I will give it a go. |
But it doesn't as long as the discriminator(s) are first in the serialization format which is usually the case. |
implemented |
I'd like to add a use case that absolutely require this. A case object type that contains an atomic can only be initialized in-place because they are non-copyable and non-movable. See: nim-lang/Nim#13093 (comment) |
In old runtime you can initialise discriminator field after variable creation like this.
In newruntime it is no longer allowed. You have to initialize object in one step using object constructor. This is the only supported way:
I was working on migration of my project to arc and I was surprised how much porting issues it causes. Serialization doesn't work, parsing doesn't work, etc.
I didn't expect it will cause so much trouble.
Now I believe that introduced case object initialization restriction made case objects unusable. A lot of code needs to be rewritten avoiding case objects completely. I would say for me now the case objects are discouraged unless you like pain.
I decided to bite the bullet and try fixing it.
Implementation details
The reason the discriminator assignment is not allowed, because the memory of
active case branch can leak. Elements in active branch needs to be destroyed before the branch switch takes place.
The idea is to generate extra destructors for discriminator fields that are capable
of destroying particular branches of the objects. This how the destructor is going to look like
for the
isStr
discriminator ofMyCaseObj
object from above.Two potential implementations possible
Discriminator as sym
Discriminators are currently syms and it will remain this way. Unfortunately syms
currently can't have destructors only types can, therefore I will have to special case
inject_destructors for discriminator sym assignments and generate destructor by
calling lift_destructors from inject_destructor on every discriminator assignment.
Discriminator as type
Discriminator will be reimplemented as a new type that will have 2 sons: underlying ordinal type and object it belongs to. Discriminator will be implicitly convertible to its underlying ordinal type, but var discriminator will not be convertible to var ordinal to maintain memory safety.
In this implementation we will can store generated destructor like for any
other type and use them from other destructors. No special casing in inject_destructors will be needed.
Caveats: Introducing a new type for discriminator is a lot of work and this kind of work I
haven't done before. It is hard for me to say how much effort it actually is and whether
it is worth it or not
The text was updated successfully, but these errors were encountered: