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

Deep copy doesn't work with with importc #19482

Open PMunch opened 2 years ago

PMunch commented 2 years ago

Discovered by user Josef over in the chat. Title says it all pretty much, types which are importc doesn't work with deepcopy.

Example

{.emit: """
typedef struct {
  float x;
  float y;
} Vector2;
""".}

type
  Vector2 {.importc.} = object
    x: cfloat
    y: cfloat
var v1 = Vector2(x: 56, y: 78)
var v2: Vector2

v2.deepCopy(v1)

assert v1 == v1
assert v2 == Vector2(x: 56, y: 78)

Current Output

Error: unhandled exception: /tmp/dctest.nim(18, 8) `v2 == Vector2(x: 56, y: 78)`  [AssertionDefect]
Error: execution of an external program failed: '/tmp/dctest '

Expected Output

No error

Version

Nim Compiler Version 1.6.2 [Linux: amd64]
Compiled at 2021-12-17
Copyright (c) 2006-2021 by Andreas Rumpf

git hash: 9084d9bc02bcd983b81a4c76a05f27b9ce2707dd
active boot switches: -d:release
iacore commented 2 years ago

Seems like the same issue, where c struct is treated as nim object in assignment with genericAssign in system/assign.nim.

Araq commented 2 years ago

For me it doesn't compile:

stdlib_system.nim.c:6713:51: error: unknown type name 'Vector2'

And it shouldn't block the release of 1.6.4

PMunch commented 2 years ago

@Araq, yes that is the issue I posted here. But this is different, I believe if you have the definition as a wrapper for an actual C library it shouldn't trigger that error and this behaviour is still present.

Araq commented 2 years ago

Look, the context here is: 1.6.4 should be out last month. Can we agree the FFI regression in 1.6.2 implies an ASAP 1.6.4 release. We can deal with your issue later, it's nasty and I'm not sure we actually support deepCopy for importc'ed types. The original design for deepCopy was never concerned with importc'ed types, not to mention that deepCopy itself is pretty much deprecated at this point -- Orc doesn't require it and it got misused for all sort of things that have nothing to do with thread interop.

PMunch commented 2 years ago

Oh for sure, I was just worried this might also be related to the FFI issues