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

Code style: decide struct initialization style #56

Open
iskra-anl opened this issue Apr 20, 2021 · 7 comments
Open

Code style: decide struct initialization style #56

iskra-anl opened this issue Apr 20, 2021 · 7 comments

Comments

@iskra-anl
Copy link
Contributor

In GitLab by @perarnau on Jan 29, 2020, 16:12

The library code is full of struct initialization statements, most notably for operators. I've noticed that there are multiple code styles used in the code, with some slight side-effects:

struct aml_struct_ops aml_specific_struct_ops = {
     .fieldname = aml_specific_struct_fieldname,
};

and

struct aml_struct_ops aml_specific_struct_ops = {
     aml_specific_struct_fieldname,
};

I would like to point out that these two styles are not equivalent: the first one allows for partial initialization of the structure, the rest of the fields having, I think, NULL as a default value, while the second one will not compile if any field is missing.

My guess is that each style might make sense for some cases, but we should make sure that the choice was made on purpose.

@iskra-anl
Copy link
Contributor Author

In GitLab by @perarnau on Jan 29, 2020, 16:12

@Kerilk and @NicolasDenoyelle , fyi.

@iskra-anl
Copy link
Contributor Author

In GitLab by @NicolasDenoyelle on Jan 30, 2020, 07:43

The more explicit, the better imo.
I would chose first style by default and use the second when it is necessary.

@iskra-anl
Copy link
Contributor Author

In GitLab by @perarnau on Jan 30, 2020, 09:12

when necessary is the big question: the first style will not fail at compile time if we forget to initialize a field in the structure. Ideally the unit tests would be enough to catch something like that, but I don't know if that's enough.

@iskra-anl
Copy link
Contributor Author

In GitLab by @perarnau on Apr 6, 2020, 15:15

FYI, clang-format will format both of those differently, in particular it will respect the one line per function style only if the fields are named...

Related to !123

@iskra-anl
Copy link
Contributor Author

In GitLab by @perarnau on Apr 6, 2020, 15:22

And apparently, the first one is called designated initializer and is a C99 feature, while the other is older.

I think I can be okay with the first style if we make sure to test properly all the methods that are mandatory.

@iskra-anl
Copy link
Contributor Author

In GitLab by @Kerilk on Apr 6, 2020, 16:05

Especially designated initializer is not completely c++ compliant. This might not be important for source code, but for headers this could become critical.

@iskra-anl
Copy link
Contributor Author

In GitLab by @perarnau on Apr 6, 2020, 17:33

We only use that on the .c side for static initialization of function pointer structs, so I think we're safe.

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

No branches or pull requests

1 participant