nim-lang / RFCs

A repository for your Nim proposals.
136 stars 23 forks source link

Case objects in new runtime removing restrictions #209

Closed cooldome closed 4 years ago

cooldome commented 4 years ago

In old runtime you can initialise discriminator field after variable creation like this.

type
  MyCaseObj = object
    case isStr: bool
      of true:  s: string
      of false: i: int

x.isStr = true
x.s = "ola"

In newruntime it is no longer allowed. You have to initialize object in one step using object constructor. This is the only supported way:

  var x = MyCaseObj(sStr: true  s: "ola")

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 of MyCaseObj object from above.

proc `=destroy`(x: var bool) = 
  var obj = cast[ptr MyCaseObj](cast[int](x.addr) - offsetof(MyCaseObj, isStr))  # container_of
  case x
  of true:
    `destroy`(obj.s)
  of false:
    `destroy`(obj.i)

Two potential implementations possible

  1. 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.

  2. 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

Araq commented 4 years ago

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?

Araq commented 4 years ago

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.

cooldome commented 4 years ago

Hi Araq, Thanks for feedback on discriminator as a type proposal, I will stick to existing sym approach. It should not be difficult to try it.

I am aware of var x = MyCaseObj(sStr: true); x.s = "ola" approach and it is similar to what I have tried. it requires accumulating all values in temporary variables during parsing and creating object at the end instead of assigning to object fields directly. It is more of porting effort question. I am rather confident it will take significantly more extra time to port stdlib because of extra limitations imposed and will slow down arc adoption considerably due to extra porting effort.

I will give it a go.

Araq commented 4 years ago

it requires accumulating all values in temporary variables during parsing and creating object at the end instead of assigning to object fields directly

But it doesn't as long as the discriminator(s) are first in the serialization format which is usually the case.

cooldome commented 4 years ago

implemented

mratsim commented 4 years ago

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: https://github.com/nim-lang/Nim/issues/13093#issuecomment-619452491