KEMBAR78
COMMON/MATH: Add some constexpr constructors to Array/List/String/Rect and the Matrix base-classes. by somaen · Pull Request #4910 · scummvm/scummvm · GitHub
Skip to content

Conversation

@somaen
Copy link
Member

@somaen somaen commented Apr 15, 2023

While working on some code that had a large amount of global variables, I wanted to get rid of the global constructor warnings that ensued once I started to use our collections/string classes in these global variables. While the globals will eventually go away in my codebase, there are also constants that would benefit from not having to run constructors.

After having a bit of a look, it seems that as long as there is a constexpr constructor available (which requires that all members are also constexpr-constructible), we can avoid global constructors.

So this is a quick attempt at adding constexpr constructors to the above mentioned classes, as well as the beginnings of supporting this in the Matrix classes as well, however the Matrix attempt is currently blocked by the logic for initializing with the identity matrix, as calling setToIdentity() triggers a warning about this being a C++14 extension.

Similarly the constructor for Common::String has to use an initializer list for the storage array, for which I've seen some comments on StackOverflow that mention that initializing arrays in the initializer list might have problems in some versions of MSVC.

This needs a fair bit of testing across compilers to be sure it doesn't break the compile on some platform, and the overall benefit boils down to being able to create compile time constants of classes that contain these classes.

somaen added 6 commits April 15, 2023 09:15
This lets us have constexpr structs/classes that contain Common::Array
This lets us have constexpr structs/classes that contain Common::List
This lets us have constexpr structs/classes that contain Rect / Point
This lets us have constexpr structs/classes that contain Common::String
This lets us have constexpr structs/classes that contain them
Currently we can't quite make Matrix3's default constructor constexpr,
as the setToIdentity-call requires C++14.
@somaen somaen requested review from aquadran and sev- April 15, 2023 07:49
@sev-
Copy link
Member

sev- commented Apr 29, 2023

Good stuff. Let's see how happy the buildbot will be.

@sev- sev- merged commit fcb8eda into scummvm:master Apr 29, 2023
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 this pull request may close these issues.

3 participants