-
Notifications
You must be signed in to change notification settings - Fork 176
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
Diplomat: delete default constructor in C++ for transparent structs #2522
Comments
Ideally we should be able to make the default constructor populate the fields with default values. But in the short term it may be advisable to delete the default constructor since it creates uninitialized memory. |
Yes, let's do that. |
Actually, wait, why is this broken? Each of these is an enum with a zero value of Auto, no? C++ ought to be generating the right default initializer here? At least, that was the intent: we do this with options bags elsewhere too. |
POD types like enums and integers are undefined until you set them in C++. |
If we want C++ to initialize the values to zero, that's code we need to write (or which Diplomat needs to generate). |
I think what we should do is:
|
Ah, right, |
Hmm, I tried doing this and it disables brace-initialization like |
@Manishearth Make sure this issue is tracked correctly. |
Background
When writing tests for Diplomat code added for Collator, a problem occurred with the new
CollatorOptions
. Currently,CollatorOptions
is an options bag struct in Rust that meant to be mutated directly before passed to a constructor. Int Diplomat, it gets transpiled to a transparent struct in C++. The generated C++ code looks like:Then, in my test code, I wrote the faulty code
"Faulty" as in, it caused segmentation faults because
ICU4XCollatorOptions options;
was not being initialized, as thevalgrind
utility pointed out.Proposal
Instead we want:
This disallows the default constructor, which forces users to initialize all fields of a transparent struct upfront when instaniating it anew. Then, they would be forced to do the safe thing in the calling code:
The text was updated successfully, but these errors were encountered: