[RFC] reset() on variant discriminators

[awr1]

According to Araq's post here and the manual here part of the design of of reset() is to allow an instance of a variant object to transition from one branch to another, something you cannot do with vanilla (i.e. =) reassignment of the discriminator by design.

In other terms, say a.kind is a discriminator in a variant and you first assign it via a.kind = Mario, it follows that any subsequent reassignment - a.kind = Luigi - would raise FieldError. To transition a.kind from Mario to Luigi, one would first need to reset(a). This cleans out the entire contents of the object, which then permits the assignment a.kind = Luigi.

The problem here is the "whole object" part. reset() is fine if that whole object is just a single case section, but Nim permits fields outside of case sections, as well as multiple case sections alongside each other, which a simple reset() cleans out entirely. We need a way to be able to reset a case section whilst preserving the remaining contents of an object.

I propose allowing reset() on case discriminators. Currently, you cannot call reset() on them, and will complain of:

variant_test.nim(17, 6) Error: type mismatch: got <Character>
but expected one of: 
proc reset[T](obj: var T)
  for a 'var' type a variable needs to be passed, but 'foo.kind' is immutable

Calling reset() on a case discriminator should do two things:

1. Reset the discriminator field (obviously)
2. Reset the complete contents of the associated union space.

Take for instance, this example:

type
  Character = enum
    Mario, Luigi, Peach, Bowser
  Thingy = object
    abc: int
    case kind: Character
    of Mario:
      def: int
      ghi: float
    of Peach:
      jkl: pointer
    else: discard

which is represented in C as:

struct Thingy {
  NI abc;
  Character kind;
  union {
    struct {
      NI def;
      NF ghi;
    } S1;
    struct {
      void* jkl;
    } S2;
  } kindU;
};

If foo were a variable of type Thingy, then calling reset(foo.kind) would internally zero out kind and the contents of kindU.

Thoughts?

[cooldome]

I like the idea.

I was also thinking of supporting the following syntax (based on the example above)

var t: Thingly

t.kind = Mario  # fails, assignment to discriminator not allowed

(t.kind, t.jkl) = (Peach, nil)   # ok, because discriminator is changed simultaneously with its value

Anyway, case objects needs a couple of improvements. reset() would be a good start

[Araq]

Nice solution. Syntactically way easier than what I antipicated.

[awr1]

@Araq Thanks.
@cooldome In your example t.kind = Mario should be legal as t.kind has yet to be assigned. With that syntax that you provide, are you proposing that an implicit reset(t.kind) occurs by (t.kind, t.jkl) = (Peach, nil)? (Maybe not for optimization purposes, and that the only area that is directly zeroed out is the area of the union space that does not overlap with the newly chosen branch (i.e. t.ghi would be reset())

[dom96]

Nice solution indeed. I like @cooldome's idea too.

[yglukhov]

Is there a case when assignment to the discriminator should not be accompanied with reset? As such, could nim rewrite discriminator assignment to changeDiscriminator(MAYBE_SOME_TYPE_INFO, &myVariantObj, newDiscriminatorValue), where changeDiscriminator would reset the discriminated fields if newDiscriminatorValue is different from the current one.

[awr1]

Not sure what you mean. You want a version of reset() that just resets the union space and then assigns the discriminator to a desired value instead of letting the discriminator be reset before we assign?

[yglukhov]

"I want" nim to automatically reset the variant object fields upon discriminant change.

type Foo = object
  case discr: bool
  of true:
    a: int
  of false:
    b: float

var f: Foo
f.b = 5.0
d.discr = true # resets implicitly
assert(f.a == 0)
f.a = 5
f.discr = false # resets implicitly
assert(f.b == 0)
f.discr = true # resets implicitly
assert(f.a == 0)
# EDIT:
f.a = 5
f.discr = true # Doesn't reset because discr doesn't change. Don't know if it's good or not though...
assert(f.a == 5)

[awr1]

Not sure that I would be in favor of hiding too much implicit behavior behind =. I suppose if you really really wanted something like this for an individual type you could define:

proc `discr=`(f: var Foo; discr: bool) =
  reset(f)
  f.`discr` = discr

Or maybe something with the experimental dot operator feature.

[Araq]

Instead we implemented #407 which solves more problems as it also addresses the (annoying) restrictions within object constructors.