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

added additional wide string support for marshalling wide strings #122

Merged
merged 12 commits into from May 19, 2022

Conversation

mwetzko
Copy link
Contributor

@mwetzko mwetzko commented May 18, 2022

added char* (C#) support for export (gen tool)
improved some code
added string-testing unit tests
some code has been formatted due to auto format

…ethodDef.Name)' again;

added char* support for easier LPStr and LPWStr marshalling
src/dnne-gen/Generator.cs Outdated Show resolved Hide resolved
src/dnne-gen/Generator.cs Outdated Show resolved Hide resolved
src/dnne-gen/Generator.cs Outdated Show resolved Hide resolved
src/dnne-gen/Generator.cs Outdated Show resolved Hide resolved
src/dnne-gen/Generator.cs Outdated Show resolved Hide resolved
src/dnne-gen/Generator.cs Outdated Show resolved Hide resolved
samples.md Outdated Show resolved Hide resolved
@AaronRobinsonMSFT AaronRobinsonMSFT added the enhancement New feature or request label May 18, 2022
mwetzko and others added 8 commits May 19, 2022 11:45
Co-authored-by: Aaron Robinson <arobins@microsoft.com>
Co-authored-by: Aaron Robinson <arobins@microsoft.com>
Co-authored-by: Aaron Robinson <arobins@microsoft.com>
Co-authored-by: Aaron Robinson <arobins@microsoft.com>
Co-authored-by: Aaron Robinson <arobins@microsoft.com>
@mwetzko mwetzko changed the title added additional wide string support for marshalling wide strings added additional wide string/booleans support for marshalling wide strings/booleans May 19, 2022
src/platform/dnne.cs Outdated Show resolved Hide resolved
src/dnne-gen/Generator.cs Outdated Show resolved Hide resolved
@mwetzko mwetzko changed the title added additional wide string/booleans support for marshalling wide strings/booleans added additional wide string support for marshalling wide strings May 19, 2022
@mwetzko
Copy link
Contributor Author

mwetzko commented May 19, 2022

I reverted the boolean commit

@AaronRobinsonMSFT AaronRobinsonMSFT merged commit e9a11da into AaronRobinsonMSFT:master May 19, 2022
@AaronRobinsonMSFT
Copy link
Owner

Thank you much! Do you need package published soon?

@mwetzko
Copy link
Contributor Author

mwetzko commented May 19, 2022

No hurry. I was looking for the boolean part to be in there. Please let me know what would be an acceptable boolean support, because at some point you will need to specify how booleans are coming in and out. Because if you do not control the part of the caller, you have to code UnmanagedType.U1 or UnmanagedType.Bool as boolean marshalling.

If I find a solution that would allow such a statement:

[UnmanagedCallersOnly]
// return bool defaults to C99 'bool'
public static bool Something(){}

[UnmanagedCallersOnly]
[return: MarshalAs(UnmanagedType.Bool)] // use Win32 'BOOL'
public static bool Something(){}

Would that be ok with you?

@AaronRobinsonMSFT
Copy link
Owner

Because if you do not control the part of the caller, you have to code UnmanagedType.U1 or UnmanagedType.Bool as boolean marshalling.

One of the main principals of DNNE is ensure there is no marshalling involved at all. This means we are simply exposing types to the C environment and as such we are required to follow what C guarantees. In this case, the size of C-bool is implementation defined. I believe since C99, even true and false are typed as ints.

This doesn't mean there is no hope, but it does mean we need to understand what guarantees we can rely on. Since bool in C# is defined to be 1 byte and uint8_t is also defined to be 1 byte, I could see an argument for the generated C export producing a uint8_t when it encounters a C# bool. However, I don't see how we could use C99's definition given it is implementation dependent.

@mwetzko
Copy link
Contributor Author

mwetzko commented May 19, 2022

I understand you to keep things as small and understandable as possible for people. But I also noticed that I'm not the only one who would like to have that boolean support. When I search the net, I can only find the C99 _Bool type which has the size of 1 byte (char size). With the <stdbool.h> header, it's now 'bool' also 1 byte. So if you ask me (not considering the VARIANT BOOL) we only have to deal with the 1 byte bool and the 4 byte BOOL.

And because we only use the "MarshalAs" to let your gen tool know what byte conversation we would like (either U1 or Bool, defaults to U1) we don't do marshalling really, just using either uint8_t or int32_t as the type for the generated C file.

Anyway, thanks for the awesome project! I guess I will use int for my BOOL conversation.

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

Successfully merging this pull request may close these issues.

None yet

2 participants