Archived

This forum has been archived. Please start a new discussion on GitHub.

Ice 3.5 slice2cs struct vs class

According to the documentation, structs will be rendered as a class in C# only if they contain "a string, proxy, class, sequence or dictionary member". However, struct is *also* being rendered as class if any of the POD members it contains have default values (line 3703 of slcie2cs/Gen.cpp). I presume this is because of the fact that structs can't have zero-argument constructors, so the defaults can't be used. This is a departure from the behavior of 3.4.x and actually leads to a bug.

When I have another struct or class that contains one or more of these would-be POD types, I get "Object reference not set to an instance of an object" exceptions thrown from the unmarshalling code of these compound objects. This is because the compound object constructors do not initialize the data members that hold these struct-like classes, so they are null when the unmarshalling code calls structDataMember.read __(is__)

You guys can fix this in several ways:

* Correctly allocate the class members that would formerly have been held by-value, solving the unmarshalling issue but likely hurting performance due to all of the extra small object allocations
* Keep the old behavior (or give the user a means of selecting it, e.g. "clr:struct" metadata) whereby the default values are ignored. This behavior is fine for me because my defaults are typically the default value for the type (i.e. 0 or 0.0).

There is a workaround to remove all of the default initializers from the Slice definitions of these types, but this means I don't get the member initialization I rely on in languages (e.g. C++) that *do* allow zero-argument constructors and I'm not really comfortable with that.

Comments

  • mes
    mes California
    Hi,

    Welcome to the forum, and thanks for reporting this.

    We recently became aware of this issue, and I've just posted a patch to correct it. This patch ensures that any Slice structure that contains default values is mapped to a C# class.

    Regards,
    Mark
  • mes wrote: »
    This patch ensures that any Slice structure that contains default values is mapped to a C# class.
    I'd prefer to see struct generated as struct as much as possible due to performance and memory overhead concerns. I use default values for POD members to ensure my C++ applications never use or send garbage data. If I remove the default values, I get the behavior I want on the C# side but not the safety I require from C++. It seems like this change is taking a step backwards.

    What about a new metadata directive e.g. "clr:struct"?
  • mes
    mes California
    Hi,

    I would have some concerns about the clr:struct metadata idea because a) it means the Slice compiler would have to ignore parts of the structure definition (the default values) and b) it would be impossible to obey in some cases, depending on the types of the data members.

    For your use case it sounds like language-specific metadata that forces data members to be zero-initialized might be a better fit.

    Mark
  • mes wrote: »
    For your use case it sounds like language-specific metadata that forces data members to be zero-initialized might be a better fit.
    That would work as well, but sounds like a much more complicated change that I think you'd want to avoid if possible. It is also a bit of a head-scratcher to me that you'd have to introduce a new C++ metadata directive because of changes to the C# code generation logic.

    I assume the rationale for the change in slice2cs behavior was to map the Slice definitions to the target language as faithfully as possible.

    * In ICE 3.4.x, C# POD-only structs would have their defaults ignored, which could be significant if those defaults are not the default value for their type. But there was a work-around available using the "clr:class" metadata directive.

    * In ICE 3.5, C# POD-only structs are always mapped to "class" if they have defaults, and removing the defaults is the only work-around (for now). But this would potentially break C++ applications because they might use uninitialized data.

    To me, reverting to the 3.4.x behavior and documenting the "clr:class" work-around for defaults requires you guys to make the fewest changes. Fewer changes = fewer chances for new bugs IMHO.