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.58k stars 1.47k forks source link

Constructing a `ref` of a `requiresInit` type causes destructions before initialization #19139

Closed alaviss closed 2 years ago

alaviss commented 2 years ago

An another case of #16607

Example

type
  O {.requiresInit.} = object
    initialized: bool

proc `=destroy`(o: var O) =
  doAssert o.initialized, "O was destroyed before initialization!"

proc initO(): O =
  O(initialized: true)

proc main() =
  let r = new O
  r[] = initO()

main()

Current Output

test.nim(15)        test
test.nim(9)         main
test.nim(6)         =destroy
lib/system/assertions.nim(38) failedAssertImpl
lib/system/assertions.nim(28) raiseAssert
lib/system/fatal.nim(53) sysFatal
Error: unhandled exception: test.nim(6, 12) `o.initialized` O was destroyed before initialization! [AssertionDefect]

Expected Output

no output

Additional Information

$ nim -v
Nim Compiler Version 1.7.1 [Linux: amd64]
Compiled at 2021-11-08
Copyright (c) 2006-2021 by Andreas Rumpf

git hash: 83a9c3ba31d180cd5e31026d8b7603bf7adea18c
active boot switches: -d:release -d:nimUseLinenoise
alaviss commented 2 years ago

The issue is that it fails with {.requiresInit.}.

While the destructors spec allows =destroy to be called on zero-initialized values, the requiresInit spec forces a strict requirement on a value to be initialized at construction. By this reasoning, it means that a value is not considered valid until it has been constructed and that no hooks can be executed before that.

In Nim, "construction" is the first value to be written into the variable (of which =sink is skipped), and the compiler already has the analysis needed for this (as required for implementation of requiresInit). This appears to me to be a case of the analysis not accounting for this specific case (it happened before in #16607).

derekdai commented 2 years ago

You are right, since compiler knows r refer to an uninitialized O, it should consider r[] = initO() is for initialize it.

Araq commented 2 years ago

There is no bug here, the spec does not mention special cases for requiresInit.