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

ARC introduces copies that can cause object slicing #17675

Open narimiran opened 3 years ago

narimiran commented 3 years ago

To reproduce this, you need to bootstrap using ARC: koch boot --gc:arc.

Example

Save the example as .rst file, and run: nim rst2html thisfile.rst

==========
Main title
==========

Some text.

.. code-block:: nim
    :test: "nim c --gc:arc $1"
  var a: array[0..1, char]
  let i = 5

More text.

Current Output

/home/miran/code/nim-lang/Nim/compiler/nim.nim(125) nim
/home/miran/code/nim-lang/Nim/compiler/nim.nim(84) handleCmdLine
/home/miran/code/nim-lang/Nim/compiler/main.nim(273) mainCommand
/home/miran/code/nim-lang/Nim/compiler/docgen.nim(1406) commandRst2Html
/home/miran/code/nim-lang/Nim/compiler/docgen.nim(1400) commandRstAux
/home/miran/code/nim-lang/Nim/lib/packages/docutils/rstgen.nim(1156) renderRstToOut
/home/miran/code/nim-lang/Nim/lib/packages/docutils/rstgen.nim(287) renderAux
/home/miran/code/nim-lang/Nim/lib/packages/docutils/rstgen.nim(1313) renderRstToOut
/home/miran/code/nim-lang/Nim/lib/packages/docutils/rstgen.nim(1010) renderCode
/home/miran/code/nim-lang/Nim/lib/packages/docutils/rstgen.nim(1520) :anonymous
/home/miran/code/nim-lang/Nim/lib/system/fatal.nim(53) sysFatal
Error: unhandled exception: invalid object conversion [ObjectConversionDefect]

Expected Output

The same .html as when there is no :test: "nim c --gc:arc $1" line. (Without that line, the example is successful)

Additional Information

Devel version of Nim bootstrapped using ARC (koch boot --gc:arc).

cc @Clyybber

Araq commented 3 years ago

Reduced example program:


type
  RstGenerator* = object of RootObj
    splitAfter*: int          # split too long entries in the TOC
    listingCounter*: int
    hasToc*: bool
    theIndex: string # Contents of the index file to be dumped at the end.
    outDir*: string      ## output directory, initialized by docgen.nim
    destFile*: string    ## output (HTML) file, initialized by docgen.nim
    filename*: string         ## source Nim or Rst file
    currentSection: string
    id*: int               ## A counter useful for generating IDs.
    onTestSnippet*: proc (d: var RstGenerator; filename, cmd: string; status: int;
                          content: string)

  TDocumentor = object of RstGenerator
    modDesc: string       # module description
    modDeprecationMsg: string

  PDoc = ref TDocumentor

proc newDocumentor*(): PDoc =
  new(result)
  result.onTestSnippet =
    proc (gen: var RstGenerator; filename, cmd: string; status: int; content: string) =
      inc(gen.id)
      var d = TDocumentor(gen)

proc call(result: PDoc) =
  result.onTestSnippet(result[], "filename", "cmd", 990, "content")

var d = newDocumentor()
d.call
Araq commented 3 years ago

Well ... ARC is correct, the docgen does some invalid object slicing here as TDocumentor is a value type. We need to fix our docgen.

Clyybber commented 3 years ago

I think the issue is that we don't copy over the RTTI in =copy:

type
  RstGenerator = object of RootObj
    filename: string

  TDocumentor = object of RstGenerator

proc test(gen: var RstGenerator) =
  gen.filename.add "he"
  echo gen of TDocumentor
  var d = TDocumentor(gen)

when false:
  # Raises ObjectConversionDefect both on arc and refc
  var r = RstGenerator()
  test(r)
else:
  # # Raises ObjectConversionDefect only on arc
  var r = TDocumentor()
  test(r)

This one shows the RTTI getting lost:

type
  RstGenerator = object of RootObj
    filename: string

  TDocumentor = object of RstGenerator

proc test2(r: var RstGenerator) =
  # With cursorinference:off shows that assignment did not copy the RTTI
  echo r of TDocumentor
  var a = r
  var b = r
  echo a of TDocumentor
var r = TDocumentor()
test2(r)

(needs to be compiled with --gc:arc --cursorinference:off)

EDIT: Hmm, nevermind, I don't think this can work with value types :D

a-mr commented 3 years ago

reduced "reduced example":

type
  A* = object of RootObj
    x*: int
    f*: proc (a: var A)

  B = object of A

proc newB*(): ref B =
  new(result)
  result.f =
    proc (a: var A) =
      inc(a.x)
      var b = B(a)

var b = newB()
b.f(b[])

I did not understand what's wrong with object slicing here. B is allocated on heap and var is passed by reference. So why is it incorrect to do var b = B(a)?

Araq commented 3 years ago

@a-mr your analysis is correct and mine was incorrect. But ARC currently constructs B(a) from a copy of a (which is of type A) and fixing this looked very hard esp considering the fringe case -- inheritance is almost always done with ref object instead. But ok, re-opening.