nim-lang / Nim

Nim is a statically typed compiled systems programming language. It combines successful concepts from mature languages like Python, Ada and Modula. Its design focuses on efficiency, expressiveness, and elegance (in that order of priority).
https://nim-lang.org
Other
16.57k stars 1.47k forks source link

Option not playing nice with ref type #16754

Open PMunch opened 3 years ago

PMunch commented 3 years ago

ARC generates bad C code for Option of a nested ref type.

Example

import options

type
  Person = ref object
    parent: Option[Person]

proc newPerson(parent: Option[Person]): Person =
  Person(parent: parent)

var person = newPerson(none(Person))

Current Output

/home/peter/.cache/nim/test_d/@mtest.nim.c: In function ‘newPerson__jROa2s0gVAb4xKj5JL9bVWw’:
/home/peter/.cache/nim/test_d/@mtest.nim.c:205:15: error: ‘tyObject_Option__41UD9alCHc2M9bOv9crGpxoLw’ has no member named ‘has’
  205 |  (*T1_).parent.has = colontmpD_.has;
      |               ^
/home/peter/.cache/nim/test_d/@mtest.nim.c:205:32: error: ‘tyObject_Option__41UD9alCHc2M9bOv9crGpxoLw’ has no member named ‘has’
  205 |  (*T1_).parent.has = colontmpD_.has;
      |                                ^

Possible Solution

Not sure what causes this, possibly related to https://github.com/nim-lang/Nim/issues/14387 and https://github.com/nim-lang/Nim/issues/9566, but this only happens for ARC. A workaround is to split the type:

import options

type
  Person = ref PersonObj
  PersonObj = object
    parent: Option[Person]

proc newPerson(parent: Option[Person]): Person =
  Person(parent: parent)

var person = newPerson(none(Person))

Additional Information

Nim Compiler Version 1.5.1 [Linux: amd64]
Compiled at 2021-01-19
Copyright (c) 2006-2021 by Andreas Rumpf

git hash: 580b8f744b288e2069792ff4181743e4e588eced
active boot switches: -d:release
cooldome commented 3 years ago

I have checked this one and it is not arc specific. typ.n received by codegen doesn't have all the fields of the struct. Looks like this problem happens earlier in the recursive type instantiation.

Clearly related to #14387 and #9566.

cooldome commented 3 years ago

The real cause is that any is ptr returns false, while correct answer is unknown it should not be evaluatable.

The Option is defined this way:

  Option*[T] = object
    when T is SomePointer:
      val: T
    else:
      val: T
      has: bool

Option[T] is being analyzed with T defined as any in liftParamType and Option[any] gets reduced to

  Option*[T] = object
      val: T
      has: bool

too early.

@Araq, I need your input on the right approach to fix it

PMunch commented 3 years ago

Not sure why you say it isn't ARC specific. It works fine with the default GC, but it appears to evaluate when T is SomePointer as false (the C code has the has field in the struct). This misses the optimisation that the pointer is nil is counted as a None but it still works. With ARC the field is gone from the struct, but the generation of the option tries to assign this field. So internally it seems to be in disagreement over whether or not T is a pointer.

Araq commented 3 years ago

@PMunch I think for a non-compiler dev it is ARC specific, but for a compiler developer that is misleading as it's not an ARC bug but a type graph bug with different consequences depending on which GC you use.

PMunch commented 3 years ago

Ah I see what you mean. So fixing it would mean ARC would work and non-ARC would actually use the pointer is nil optimisation, both of which I assume have the same underlying cause.

moigagoo commented 2 years ago

This issue bit me recently in the context of defining a Norm model: https://forum.nim-lang.org/t/8853

bung87 commented 2 years ago

maybe similar to https://github.com/nim-lang/Nim/issues/14758