[ARC] Global variable declared in a block is destroyed too early

[khchen]

Following code works well before the update: An optimizer for ARC (#14962).
Compiling with --gc:arc.

Example

import wNim/[wApp, wFrame, wButton]

let app = App()
let frame = Frame()
let button = Button(frame)

frame.wEvent_Size do ():
  frame.autolayout """
    HV:|[button]|
  """

frame.show()
frame.center()
app.mainLoop()

Current Output

Traceback (most recent call last)
O:\nim\wnim\release\test3.nim(14) test3
O:\nim\wnim\release\wNim\private\wApp.nim(304) mainLoop
O:\nim\wnim\release\wNim\private\wApp.nim(296) messageLoop
O:\nim\wnim\release\wNim\private\wWindow.nim(2262) wWndProc
O:\nim\wnim\release\wNim\private\wWindow.nim(2262) wWndProc
O:\nim\wnim\release\wNim\private\wWindow.nim(2262) wWndProc
O:\nim\wnim\release\wNim\private\wWindow.nim(2250) wWndProc
O:\nim\wnim\release\wNim\private\wWindow.nim(1289) processMessage
O:\nim\wnim\release\wNim\private\wWindow.nim(1283) processMessage
O:\nim\wnim\release\wNim\private\wWindow.nim(1240) processEvent
O:\nim\wnim\release\wNim\private\wWindow.nim(1222) callHandler
O:\nim\wnim\release\wNim\private\wWindow.nim(1971) wWindow_DoSize
O:\nim\wnim\release\wNim\private\wWindow.nim(1297) processMessage
O:\nim\wnim\release\wNim\private\wWindow.nim(1289) processMessage
O:\nim\wnim\release\wNim\private\wWindow.nim(1283) processMessage
O:\nim\wnim\release\wNim\private\wWindow.nim(1252) processEvent
O:\nim\wnim\release\wNim\private\wWindow.nim(1226) callHandler
O:\nim\wnim\release\wNim\private\wResizable.nim(234) :anonymous
O:\nim\wnim\release\wNim\private\wResizer.nim(92) resolve
O:\nim\wnim\release\wNim\private\kiwi\solver.nim(273) updateVariables
SIGSEGV: Illegal storage access. (Attempt to read from nil?)

Thanks to @Yardanico, the simplified code is

type
  Resizer = ref object
    data: string

proc layout() =
  block:
    var resizer {.global.}: Resizer
    if resizer == nil:
      resizer = Resizer(data: "hi")

    discard resizer.data == "hi"

layout()
layout()

$ nim -v
Nim Compiler Version 1.3.5 [Windows: amd64]
Compiled at 2020-07-17
Copyright (c) 2006-2020 by Andreas Rumpf

git hash: 6b607413e90111f74dd77c1fa31ba5f183f72bdf
active boot switches: -d:release

[Araq]

Cannot reproduce.

[Araq]

If you cannot minimize, please provide a different program that also fails for wNim, hopefully it crashes on my machine too then.

[ghost]

Can reproduce in Wine on latest devel :) Seems like it's not a cursorifier bug (crashes with cursorifier disabled)

[khchen]

After removing all macros and templates, the code looks like this. It works without --gc:arc, but crashes with --gc:arc.

import wNim/[wApp, wFrame, wButton, wResizer]

let app = App()
let frame = Frame()
let button = Button(frame)

proc layout() =
  block:
    var resizer {.inject, global.}: wResizer
    if resizer == nil:
      resizer = Resizer(frame)
      resizer.addObject(button)
      resizer.addConstraint(frame.mLeft == button.mLeft)
      resizer.addConstraint(frame.mTop == button.mTop)
      resizer.addConstraint(frame.mWidth == button.mWidth)
      resizer.addConstraint(frame.mHeight == button.mHeight)

    resizer.resolve()
    resizer.rearrange()

frame.wEvent_Size do ():
  layout()

layout()
frame.show()
frame.center()
app.mainLoop()

However, these code always works:

proc layout() =
  # block:
    var resizer {.inject, global.}: wResizer

proc layout() =
  block:
    var resizer {.inject.}: wResizer

In summary, the problem is related to block and global.
I hope someone can reproduce these results.

[ghost]

@khchen thanks for the simplification, it allowed me to repro it without any dependencies, can you please rename the issue (to something like "[ARC] Global variable declared in a block is destroyed too early") and add the repro to the first post ? :)

type
  Resizer = ref object
    data: string

proc layout() =
  block:
    var resizer {.global.}: Resizer
    if resizer == nil:
      resizer = Resizer(data: "hi")

    discard resizer.data == "hi"

layout()
layout()

expandArc:

block :tmp:
  var resizer
  if resizer == nil:
    `=sink`(resizer, Resizer(data: "hi"))
  discard resizer.data == "hi"
  `=destroy`(resizer)

The issue seems to be that the compiler doesn't recognize (or "forgets") about global pragma of a variable if it's in a block.

[Clyybber]

This bug is caused by this line: https://github.com/nim-lang/Nim/blob/devel/compiler/injectdestructors.nim#L530
Changing it to remove the weird parent restriction makes this example work, but makes another test fail.
IMO {.global.} has extremely weird and underspecified semantics and should be deprecated; its easy to use them to create invalid C code simply by declaring a {.global.} in a proc and making its value depend on one of the proc args. With --gc:arc this might actually work, because they get created like normal variables on each call of the proc.

I think they are a minefield and I don't really see a usecase for them where a normal top-level global wouldn't do it (except for the part that they are initialized before those other globals, but maybe a {.earlyInit.} pragma would be better served for that purpose).

[khchen]

Similar problem:

type
  T = ref object
    data: string

template foo(): T =
  var a {.global.}: T
  once:
    a = T(data: "hi")
    echo "init"

  a

proc test() =
  var a = foo()
  echo a.data

test()
test()

Compile without --gc:arc

init
hi
hi

Compile with --gc:arc

SIGSEGV: Illegal storage access. (Attempt to read from nil?)

Nim Compiler Version 1.4.0 [Windows: amd64]
Compiled at 2020-10-18
Copyright (c) 2006-2020 by Andreas Rumpf