Skip to content
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

Option not playing nice with ref type #16754

Open
PMunch opened this issue Jan 19, 2021 · 7 comments
Open

Option not playing nice with ref type #16754

PMunch opened this issue Jan 19, 2021 · 7 comments

Comments

@PMunch
Copy link
Contributor

PMunch commented Jan 19, 2021

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 #14387 and #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
Copy link
Member

cooldome commented Jan 21, 2021

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
Copy link
Member

cooldome commented Jan 25, 2021

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
Copy link
Contributor Author

PMunch commented May 11, 2021

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
Copy link
Member

Araq commented May 11, 2021

@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
Copy link
Contributor Author

PMunch commented May 11, 2021

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.

@ringabout ringabout changed the title ARC not playing nice with Option of ref type Option not playing nice with ref type Sep 13, 2021
@moigagoo
Copy link
Contributor

moigagoo commented Feb 9, 2022

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

@bung87
Copy link
Collaborator

bung87 commented Oct 22, 2022

maybe similar to #14758

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants