Incidencia #43826

CodingStyle: compound literals

Abrir Fecha: 2022-02-09 01:20 Última actualización: 2022-02-15 19:18

Informador:
Propietario:
Tipo:
Estado:
Cerrado
Componente:
Hito:
Prioridad:
5 - Medium
Gravedad:
5 - Medium
Resolución:
Fixed
Fichero:
1

Details

Continued from #43809.

Compound literals (which look similar to struct/unition initializers) are currently only used in very few places (most prominently, the requirement_fulfilled_by_* macros in common/requirements.h). We need to figure out / decide if and how we generally want to use them, and add that information to doc/CodingStyle.

Ticket History (3/14 Histories)

2022-02-09 01:20 Updated by: alienvalkyrie
  • New Ticket "CodingStyle: compound literals" created
2022-02-09 01:51 Updated by: alienvalkyrie
Comentario

There are two big questions here:

  • For using them in general; is the issue with certain compilers (for C++ in particular?) not supporting them still a thing?
  • For using them in the specific use case of #43809; can we depend on omitted pointer-valued struct members being initialized as NULL correctly in all cases (all platforms, all compilers)?

I don't really know about the first question. If I had to guess, I'd assume most problems with that will have been solved or worked-around by now, given that compound literals have been in use for years now.

Re: the second question – as far as I can tell without purchasing the official standards document from ISO, compound literal initialization works the same as struct initialization (1), where omitted members are zero-initialized (2), and zero-initialized pointers are set to appropriate null pointers (3), so the answer would be "yes".

2022-02-09 02:07 Updated by: cazfi
Comentario

I suspect those uses that exist are relatively new code? While the CodingStyle has not said anything against them, I've believed that their use is to be avoided for compatibility reasons. Some research on their support should be done. Especially if Visual Studio now supports them, and what is the first version that does.

It's also possible that we end allowing their use in specific contexts, e.g., if some compiler implements them partially. Like tcc does vararrays (which we may have not properly documented, the thing just came up recently when we had to revert strict compiler vararrays compliance check to one that checks just the specific case that freeciv uses).

2022-02-09 02:29 Updated by: alienvalkyrie
Comentario

Reply To cazfi

I suspect those uses that exist are relatively new code?

Almost eight years old; first introduced in what is now commit 0076666. Perhaps relatively new by your standards, not very new by my standards.

2022-02-09 02:58 Updated by: cazfi
Comentario

Reply To alienvalkyrie

Reply To cazfi

I suspect those uses that exist are relatively new code?

Almost eight years old; first introduced in what is now commit 0076666

Existed in 2.6 stable series, and nobody has complained.

2022-02-13 06:07 Updated by: alienvalkyrie
Comentario

Reply To cazfi

Existed in 2.6 stable series, and nobody has complained.

Does that mean we're going forward with officially allowing them? If so, should we add appropriate configure checks? (And by "we" I mean "you", because I have no clue how those work.)

2022-02-13 06:51 Updated by: cazfi
Comentario

I still don't know if latest version of Visual Studio supports them or not. Whether that's relevant is another question. Will there ever be anyone making it possible to build freeciv with Visual Studio? (This might have the usual answer: I've ordered a new Windows laptop for making windows builds)

Yes, if we are going to allow them, we should add configure checks, at least for the autotools builds, maybe also for the meson builds.

2022-02-14 01:52 Updated by: alienvalkyrie
Comentario

Reply To cazfi

I still don't know if latest version of Visual Studio supports them or not.

Apparently, based on answers I got from people I asked, the Microsoft Visual C++ compiler supports compound literals in C mode since MSVC 2013 (source). (It does not seem to support compound literals in C++ mode.)

So that sounds like a "yes", assuming the distinction between "C mode" and "C++ mode" doesn't need to happen on a project level, but on a compilation unit level.

Whether that's relevant is another question.

I'd argue that when push comes to shove, code readability should be considered more important than compatibility with a proprietary development environment. There would certainly be upsides to supporting VS, but I don't think they're greater than the upsides of being able to declare and initialize a struct that's only used in one place in the place it's used.

2022-02-14 02:04 Updated by: alienvalkyrie
Comentario

Reply To alienvalkyrie

supports compound literals in C mode since MSVC 2013 (source).

Note: According to that source, designated initialization (which is already used in plenty of places, e.g. to initialize universals and citizen governor presets) is also only supported since MSVC 2013; so if I'm reading this correctly (again, depending on the mode distinction thing from above), using compound literals wouldn't make our code any less MSVC-compatible than it was before.

2022-02-14 02:09 Updated by: cazfi
Comentario

Please write the CodingStyle patch, and open ticket about the configure check.

2022-02-14 02:40 Updated by: alienvalkyrie
Comentario

Reply To cazfi

Please write the CodingStyle patch, and open ticket about the configure check.

Done and done #43855

2022-02-14 02:40 Updated by: alienvalkyrie
  • Propietario Update from (Ninguno) to alienvalkyrie
  • Resolución Update from Ninguno to Accepted
2022-02-15 19:18 Updated by: alienvalkyrie
  • Estado Update from Open to Cerrado
  • Resolución Update from Accepted to Fixed

Editar

You are not logged in. I you are not logged in, your comment will be treated as an anonymous post. » Entrar