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

Some signatures (involving out parameters) seem to be wrong #47

Closed
TChatzigiannakis opened this issue Apr 26, 2016 · 1 comment
Closed
Assignees
Labels

Comments

@TChatzigiannakis
Copy link
Contributor

TChatzigiannakis commented Apr 26, 2016

I think some of the signatures generated by the script are wrong. I will explain why I think this is the case, but I'd like someone else to validate it as well, as I haven't gotten around to testing it yet.

Consider the example of LLVMDiagnosticHandler. On the C side, it is defined as:

typedef void (*LLVMDiagnosticHandler)(LLVMDiagnosticInfoRef, void *);

Where LLVMDiagnosticInfoRef is:

typedef struct LLVMOpaqueDiagnosticInfo* LLVMDiagnosticInfoRef

Because of this, I'm guessing LLVMDiagnosticHandler is seen by the script as:

typedef void (*LLVMDiagnosticHandler)(LLVMOpaqueDiagnosticInfo*, void *);

And the script emits the following delegate type:

[UnmanagedFunctionPointer(CallingConvention.Cdecl)]
public delegate void LLVMDiagnosticHandler(out LLVMOpaqueDiagnosticInfo @param0, IntPtr @param1);

On the C++ side, setting the diagnostic handler simply saves the passed arguments and, at a later point, another function calls the function pointer and passes a const DiagnosticInfo& to it. I think this means that the C++ side expects that first parameter to be an input parameter passed by reference, while the C# side assumes it is an output parameter.

If this is the case, an LLVMSharp user might not be able to create a useful handler, because the C# compiler will force any handler to overwrite the out parameter explicitly (or the C# compiler will insert an implicit overwrite). This means that anything passed to the handler from the C++ side will be lost (and the handler could even corrupt the state of the C++ side). Changing the script to generate ref instead of out should fix that.

I don't know how many other types may have this issue -- this simply happened to be the first one I attempted to work on.

@mjsabby
Copy link
Contributor

mjsabby commented Sep 16, 2017

Fixed in eeb7e37

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants