Implement easy C externs#1402
Conversation
This reverts commit c7fa34f.
|
@b-studios ping |
b-studios
left a comment
There was a problem hiding this comment.
Great work!
What I would love to see is a simple test, but I understand that might be difficult to integrate into our testing infrastructure?
| emit(Call("ccall", Ccc(), transformedReturnType, ConstantGlobal(cFunctionName), transformedParameters.map { case Parameter(typ, name) => LocalReference(typ, name) })) | ||
|
|
||
| transformedReturnType match { | ||
| case VoidType() => RetVoid() | ||
| case _ => Ret(LocalReference(transformedReturnType, "ccall")) |
There was a problem hiding this comment.
This is always just generating a single call, right? What is the purpose of the defineCWrapperFunction then? It is only called here, right?
There was a problem hiding this comment.
Just readability and consistency with defineFunction and defineLabel, but it is technically only called here. Yes.
| case (machine.Type.CTpe(machine.CType.Void), machine.Type.Positive()) => | ||
| () | ||
| case (machine.Type.Positive(), machine.Type.CTpe(machine.CType.Void)) => | ||
| () |
There was a problem hiding this comment.
Do these show up? This sounds dangerous since it violates that we should not use C::Void anywhere
There was a problem hiding this comment.
The coercion also happens to return values of functions.
As we cannot coerce void or even pass it to a function as an argument we cannot use the existing infrastructure and need to handle it somewhere earlier. So there were two possible solutions.
- Special case
CTpe(Void)in machine transformerperhapsUnboxand never generate the coercion - Special case here and do this no-op coercion
I thought the latter looked slightly better
| case Type.Byte() => "Byte" | ||
| case Type.Double() => "Double" | ||
| case Type.Reference(tpe) => toDoc(tpe) <> "*" | ||
| case Type.CTpe(name) => s"CType(${name})" |
There was a problem hiding this comment.
Probably, I would just print name here. But this really does not matter :)
| case Byte() | ||
| case Double() | ||
| case Reference(tpe: Type) | ||
| case CTpe(tpe: CType) |
There was a problem hiding this comment.
We should bikeshed this constructor name here. Same actually for CType. The strings on the right hand side of a c extern actually are llvm type names. So CType is a bit of a misnomer, or am I wrong?
There was a problem hiding this comment.
Yes. I thought more along the lines of types for c externs instead of c types directly, but I am open to suggestions
Co-authored-by: Jonathan Immanuel Brachthäuser <jonathan.brachthaeuser@uni-tuebingen.de>
More in line with the LLVM naming scheme
|
To reliably test this I would need to link against some c file in the testing environment. This leads back to the config file / command line debate and I would rather not hack around that for now. |
Enables writing extern definitions such as
which has to match a linked c function that has the correct name and types.
In this specific case
This will automatically generate a wrapper function for said C function, which can then be called from effekt.
There are some selected types and utility functions in
effekt.effekt