David-Kunz commented 3 years ago

Make `fields(x, y)` and `fieldPairs(x, y)` work for case objects

Abstract

fields(x, y) and fieldPairs(x, y) should also work for case objects and should iterate over the intersection of both variants.

Motivation

fields(x, y) and fieldPairs(x, y) are for example used for comparisons.

The default implementation for the == procedure involves the usage of fields(x, y). If you try to compare case objects you get the error message Error: parallel 'fields' iterator does not work for 'case' objects. This could scare new Nim users.

Especially in unit tests, the ability to compare case objects is very important.

To solve this problem, you can overwrite the == function but that involves complicated macros or hardcoded solutions.

Description

The solution is to enhance fields(x, y) to also work with case objects: Iterate over the intersection of both variants.

Note: Variants share at least one common field (the kind discriminator), so the intersection is never empty and the == comparison is only true if the variants match.

Examples

Before

Consider this example:

type
  MyEnum = enum
    VarA, VarB
  MyObj = object
    case kind: MyEnum
    of VarA: fieldA: string
    of VarB: fieldB: string

let myA = MyObj(kind: VarA, fieldA: "variant A")
let myB = MyObj(kind: VarB, fieldB: "variant B")

if myA == myB: # Error: parallel 'fields' iterator does not work for 'case' objects
  echo: "objects are equal"

The current solution for the above problem would be:

func `==`(a, b: MyObj): bool =
  result = true
  if a.kind != b.kind:
    return false
  case a.kind:
    of VarA:
      if a.fieldA != b.fieldA:
        return false
    of VarB:
      if a.fieldB != b.fieldB:
        return false

After

(Consider the same example)

fields(myA, myB) would iterate over the intersection, in this case only the kind field, as they don't have any other fields in common. As kind differs, myA == myB would be equal to false.

haxscramper commented 3 years ago

Because fieldPairs unrolls for loop into sequence of statements it could work the same way for case objects, just creating if checks for guarded fields. So example from the RFC

type
  MyEnum = enum
    VarA, VarB, VarC
  MyObj = object
    case kind: MyEnum
    of VarA: fieldA: string
    of VarB, VarC: fieldB: string
    fieldD: float

Should generate the following code for parallel pairs (almost like it does it with regular objects but just account for parallel fields to).

if obj1.kind in {VarA} and obj2.kind in {VarA}: # Both objects are in the same branch
  # execute loop body
elif obj1.kind in {VarB, VarC} and obj2.kind in {VarB, VarC}: # Other branch
  # loop body

# loop body for `kind` ! - discriminanti field itself is also a perfectly valid field.
# loop body for fieldD

Ability to iterate on object fields is also important for providing default implementations for tree diffing. I've tried implementing this using macro transformation but int the end it turned out to be a complete mess that requires to constantly getTypeImpl, unpack things over and over again and breaks in all possible ways when you use private fields.

In addition to just being generally useful it is also kind of annoying to get this unreadable error message with field pairs does not work on case objects that doesn't even mention the type of the object. In a lot of cases variant objects start as regular ones, then you get more and more edge case that eventually demand the object to be a variant.

Proposal by @Clyybber (in https://github.com/nim-lang/RFCs/issues/19#issuecomment-633684323) would not create any new complications as it would only mean we would iterate over first group of case fields, then over second, and so on. Not different from having multiple case fields (which is already supported)

shirleyquirk commented 3 years ago

i did this once in a forum post i'll dig it up yes, big ol macro and helper procs https://forum.nim-lang.org/t/6781#42294 just as you said @haxscramper, lots of getTypeImpl

seems to work even if the fields are private and in a separate file,tho. yes, apparantly macros/getTypeImpl can completely bypass privacy

haxscramper commented 3 years ago

seems to work even if the fields are private and in a separate file,tho. yes, apparantly macros/getTypeImpl can completely bypass privacy

getTypeImpl can bypass privacy, but generated code can't. fieldPairs on the other hand allows getting into private fields when needed.

I inject fieldPairs for single object iteration when I need to compare two private fields. So for getting private field I generate the following code:

      let rhs: bool = block:
        var tmp: bool
        for name, val in fieldPairs(rhsObj):
          when val is bool:
            if name == "kind":
              tmp = val
              break
        tmp

Araq commented 3 years ago

Sounds like a reasonable, well-thought out proposal that is less disruptive than my own ideas.

nim-lang / RFCs