std::string inside of value types does not generate correct bindings for C#

[mirkobraic]

Hello, I have a C++ class that I want to expose to C# as a value type using CS_VALUE_TYPE macro.

However, I have noticed that if my C++ class contains std::string or const char* member, the generated code is not correct and cannot build. For example, consider the following C++ code:

CS_VALUE_TYPE class EXPORT_MACRO value_type {
public:
  std::string string_member;
  const char *char_ptr_member;
};

It generates public unsafe partial struct ValueType C# structure with the following code for member variables:

public string StringMember
{
    get
    {
        var __basicStringRet0 = global::Std.BasicString<sbyte, global::Std.CharTraits<sbyte>, global::Std.Allocator<sbyte>>.__CreateInstance(__instance.string_member);
        return global::Std.BasicStringExtensions.Data(__basicStringRet0);
    }

    set
    {
        global::Std.BasicStringExtensions.__Internal.Assign(new __IntPtr(&__instance.string_member), value);
    }
}

public string CharPtrMember
{
    get
    {
        return CppSharp.Runtime.MarshalUtil.GetString(global::System.Text.Encoding.UTF8, __instance.char_ptr_member);
    }

    set
    {
        if (___instance.char_ptr_member_OwnsNativeMemory)
            Marshal.FreeHGlobal(__instance.char_ptr_member);
        ___instance.char_ptr_member_OwnsNativeMemory = true;
        if (value == null)
        {
            __instance.char_ptr_member = global::System.IntPtr.Zero;
            return;
        }
        var __bytes0 = global::System.Text.Encoding.UTF8.GetBytes(value);
        var __bytePtr0 = Marshal.AllocHGlobal(__bytes0.Length + 1);
        Marshal.Copy(__bytes0, 0, __bytePtr0, __bytes0.Length);
        Marshal.WriteByte(__bytePtr0 + __bytes0.Length, 0);
        __instance.char_ptr_member = (__IntPtr) __bytePtr0;
    }
}

StringMember setter fails with "CS0212 You can only take the address of an unfixed expression inside of a fixed statement initializer" error message, while CharPtrMember setter fails with "CS0212 The name '___instance' does not exist in the current context".

Do you know what could be the issue and how to fix it?

P.S.
The generated code works correctly if CS_VALUE_TYPE macro is omitted, but unfortunately, I need to use C# structs.
Here is an example that works:

public string StringMember
{
    get
    {
        var __basicStringRet0 = global::Std.BasicString<sbyte, global::Std.CharTraits<sbyte>, global::Std.Allocator<sbyte>>.__CreateInstance(new __IntPtr(&((__Internal*)__Instance)->string_member));
        return global::Std.BasicStringExtensions.Data(__basicStringRet0);
    }

    set
    {
        global::Std.BasicStringExtensions.__Internal.Assign(new __IntPtr(&((__Internal*)__Instance)->string_member), value);
    }
}

public string CharPtrMember
{
    get
    {
        return CppSharp.Runtime.MarshalUtil.GetString(global::System.Text.Encoding.UTF8, ((__Internal*)__Instance)->char_ptr_member);
    }

    set
    {
        if (__char_ptr_member_OwnsNativeMemory)
            Marshal.FreeHGlobal(((__Internal*)__Instance)->char_ptr_member);
        __char_ptr_member_OwnsNativeMemory = true;
        if (value == null)
        {
            ((__Internal*)__Instance)->char_ptr_member = global::System.IntPtr.Zero;
            return;
        }
        var __bytes0 = global::System.Text.Encoding.UTF8.GetBytes(value);
        var __bytePtr0 = Marshal.AllocHGlobal(__bytes0.Length + 1);
        Marshal.Copy(__bytes0, 0, __bytePtr0, __bytes0.Length);
        Marshal.WriteByte(__bytePtr0 + __bytes0.Length, 0);
        ((__Internal*)__Instance)->char_ptr_member = (__IntPtr) __bytePtr0;
    }
}

[Saalvage]

Fixing the generation was the easy part, however, there seems to be something wrong with how the strings work. After writing once you get the pleasure of continuously reading garbage.
See https://github.com/Saalvage/CppSharp/tree/fix/struct-string-member / Saalvage@3fd7ee4 for a fix of the codegen.

Not entirely sure how to tackle this one atm.

[tritao]

Thanks for looking into this, might be worth getting the generation fixes in at least. Can't really justify spending time trying outside that myself atm fixing this, maybe we just merge your work and ignore the test for now. Can we easily generate code to throw an exception for this broken case?

[Saalvage]

Found the issue: CppSharp does not generate a default constructor for value types, even if one is explicitly defined on the C++ side. This causes the string to not be initialized correctly, causing everything to break down later on. Providing a dummy empty constructor taking an int and calling it in C# makes everything work as expected.
I assume this is by design because you can't force a struct to use it's constructor, which makes this entire situation rather dubious.

I'm not entirely sure what the best course of action here is, since you can't force ctors to be called on C# structs (default always zero initializes)..
Either:

1. We simply create the ctor on the C# side and if the user doesn't call it that's their fault.
2. We utilize an _initialized variable, which calls the ctor if anything is done on the object without it being initialized before.
   Maybe a config option to toggle between the two? Since the latter has slighty more overhead when using the struct (is that even a concern? Not like interop is particularly fast anyways).

Either way, I think this calls for a new issue, after the codegen here is fixed.

[tritao]

Found the issue: CppSharp does not generate a default constructor for value types, even if one is explicitly defined on the C++ side. This causes the string to not be initialized correctly, causing everything to break down later on. Providing a dummy empty constructor taking an int and calling it in C# makes everything work as expected. I assume this is by design because you can't force a struct to use it's constructor, which makes this entire situation rather dubious.

Good find.

2. We utilize an _initialized variable, which calls the ctor if anything is done on the object without it being initialized before.
Maybe a config option to toggle between the two? Since the latter has slighty more overhead when using the struct (is that even a concern? Not like interop is particularly fast anyways).

This sounds ok to me, either initializing it or just throwing if the struct is not initialized. I'd rather not add more options as we already have quite a lot of them, but if the overhead is something anyone particularly cares about I'd be ok with it. Probably not necessary tho?