Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cdecl proc mistakenly gets a pointer in generated C code (FFI, no bycopy) #1908

Closed
oprypin opened this issue Jan 8, 2015 · 2 comments
Closed

Comments

@oprypin
Copy link
Contributor

oprypin commented Jan 8, 2015

type Vertex = object
    position: Vector2f
    color: Color
    texCoords: Vector2f

type VertexArray = ptr object

proc append(vertexArray: VertexArray, vertex: Vertex) {.
  cdecl, dynlib: "libcsfml-graphics.so", importc: "sfVertexArray_append".}
Dl_106214 = (TY106215) nimGetProcAddr(TMP163, "sfVertexArray_append");

typedef N_CDECL_PTR(void, TY106215) (vertexarrayHEX3Aobjecttype104444* vertexarray, 
                                     vertex105041* vertex);

triangle.append v1  # triangle is a VertexArray, v1 is a Vertex
Dl_106214(triangle_110022, (&v1_110028));

As you can see, the function signature in generated C code has the 2nd argument as a pointer, while Vertex is a normal object. As I've been told, this may be caused by a lack of {.bycopy.} on Vertex; Nim decides it wants to pass such a big object by a pointer. But it is unacceptable to modify the signature of a function if it is part of C FFI.

It was extremely hard to track down the problem, because the code compiled and worked without crashing, just wrote pointers where data was expected...

@Araq
Copy link
Member

Araq commented Jan 8, 2015

But it is unacceptable to modify the signature of a function if it is part of C FFI.

Er no? But ok, what makes it part of the C FFI? 'cdecl'? Hardly. 'dynlib'? What if it's dynlib imported from Nim?

oprypin added a commit to oprypin/nim-csfml that referenced this issue Jan 8, 2015
@Araq
Copy link
Member

Araq commented Mar 12, 2015

Well we cannot change the FFI at this point, the breaking change would be enormous and it's not a bug but documented behaviour.

@Araq Araq closed this as completed Mar 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants