godot icon indicating copy to clipboard operation
godot copied to clipboard

Core: Utilize initalizer lists in Math constructors

Open Repiteo opened this issue 10 months ago • 3 comments

The Math equivalent of #90866, whereby the bulk of math constructors set their properties via initalizer lists. Differs from Variant in two main ways:

  1. Not all constructors got this treatment, as some had more advanced logic that was outside the scope of this conversion.
  2. Due to using exclusively primitive types, the changed constructors are all able to safely use constexpr. (Possible groundwork for constexpr math types? 🤔)

Repiteo avatar Apr 24 '24 00:04 Repiteo

Wait, does making the constructors constexpr qualify the structs for unused checks? That's... Interesting

Repiteo avatar Apr 24 '24 01:04 Repiteo

Just to confirm, it looks like compiler is pretty good at optimizing out unneeded constructors even without initializer list, even at minimal optimization -O1. This is trivial case though, there might be some cases where there is benefit.

https://godbolt.org/z/cYWed7fWK

This does suggest that in most cases these changes are a style preference issue, rather than functional change. However the constexpr could be useful.

lawnjelly avatar Apr 24 '24 07:04 lawnjelly

Making these constexpr revealed a strange conflict with unions: clang-tidy wants structs inside unions to have default member initalization, but a union only allows a single field initalizer and that logic includes the internals of a struct. So as of now, the default values must be part of their respective coord array, instead of being assigned to the named variables; though, the named variables end up being what's assigned in the member initializers. Trying to alter the unions themselves is probably outside the scope of this PR, but I'll test a bit and see how invasive it would end up being; most likely outcome: it'll be part of an immediate followup PR.

Repiteo avatar Apr 24 '24 14:04 Repiteo

Superseded by #91992.

Repiteo avatar May 15 '24 18:05 Repiteo