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

Value types do not generate a default constructor #1777

Closed
Saalvage opened this issue Oct 19, 2023 · 3 comments · Fixed by #1783
Closed

Value types do not generate a default constructor #1777

Saalvage opened this issue Oct 19, 2023 · 3 comments · Fixed by #1783

Comments

@Saalvage
Copy link
Contributor

The following code:

CS_VALUE_TYPE class DLL_API value_type {
public:
    std::string string_member;
};

does not generate a default constructor on the C# side of things, meaning it is impossible to initialize the object correctly and causing uninitialized memory reads after a write.

See #1730 for usecase.

@tritao
Copy link
Collaborator

tritao commented Oct 20, 2023

@Saalvage
Copy link
Contributor Author

Oh, I totally forgot that this didn't use to be a feature. So this is most likely the reason why this wasn't previously implemented. This would definitely make it work in the "happy" case, however, you can still zero-initialize structs via various other means (as mentioned in that article). Preventing that would require generating code for checking if the struct is initialized in every single method called on the struct, I'm not sure if that would be worth it or if it may just be enough to tell people to call their ctors.

@JordanL8
Copy link

So this solves the problem of calling native constructors from C# structs, however explicit parameterless constructors for structs are I think a C# 10.0 feature.

For users on lower C# versions, is there an easy way to opt out of generating them? I'm personally explicitly ignoring each, which doesn't cause issues because these structs represent C++ PoD classes that don't have explicit constructors. I only have 7 or so to ignore so I could imagine this could be a faff for users who have a lot more.

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

Successfully merging a pull request may close this issue.

3 participants