search

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

VM crash with object variant, adding seq to seq field #17039

Closed metagn closed 3 years ago

metagn commented 3 years ago

Encountered in nimscript. The issue seems to be that fields in object variant branches break when converting to var T.

Example

type
  Obj1 = object
    case kind: bool
    of false:
      field: seq[int]
    else: discard

static:
  var obj1 = Obj1()
  obj1.field.add(@[])

Current Output

....compiler\nim.nim(119) nim
compiler\nim.nim(84) handleCmdLine
compiler\main.nim(242) mainCommand
compiler\main.nim(213) compileToBackend
compiler\main.nim(90) commandCompileToC
compiler\modules.nim(177) compileProject
compiler\modules.nim(97) compileModule
compiler\passes.nim(180) processModule
compiler\passes.nim(73) processTopLevelStmt
compiler\sem.nim(607) myProcess
compiler\sem.nim(575) semStmtAndGenerateGenerics
compiler\semstmts.nim(2306) semStmt
compiler\semexprs.nim(1029) semExprNoType
compiler\semexprs.nim(2865) semExpr
compiler\semstmts.nim(2248) semStmtList
compiler\semexprs.nim(2917) semExpr
compiler\semstmts.nim(2204) semStaticStmt
compiler\vm.nim(2213) evalStaticStmt
compiler\vm.nim(2202) evalConstExprAux
compiler\vm.nim(716) rawExecute
lib\system\fatal.nim(53) sysFatal
Error: unhandled exception: 'node' is not accessible using discriminant 'kind' of type 'TFullReg' [FieldDefect]

Expected Output

compiles

Possible Solution

As an immediate solution, if you change this line in opcLdObj:

https://github.com/nim-lang/Nim/blob/6e267d28b3459aa447cae07ba61209438977cd5c/compiler/vm.nim#L714

to this line below it in opcLdObjAddr:

https://github.com/nim-lang/Nim/blob/6e267d28b3459aa447cae07ba61209438977cd5c/compiler/vm.nim#L730

it works. Maybe the wrong VM opcode is getting generated, but it's better to be safe than crashing the compiler.

Additional Information

$ nim -v
Nim Compiler Version 1.5.1 [Windows: amd64]
Compiled at 2021-02-14
Copyright (c) 2006-2021 by Andreas Rumpf

git hash: fff5819ee728aa62fd857982f799ecc0fac0fd5e
saem commented 3 years ago

I compared the transforms of this with and without the kind, turns out the access of field is wrapped in an nkCheckedFieldExpr in one and not the other. the analyseIfAddressTaken proc in semexprs.nim handles that so I added an unwrap case (doesn't feel a 100% right) but seems less terrible than putting an if guard for poor code gen. 🤷🏽

PR forth coming.