DeviceEvent shouldn't use C++20 initializers

[ksperling-apple]

#32935 introduced C++20 aggregate initializer syntax for DeviceEvent objects in various places. That's not ideal in a C++17 code base and doesn't match the code style used everywhere else.

Can we define constexpr factory functions for each event type instead that set the type and the relevant part of the union?

[bzbarsky-apple]

@mwswartwout @andy31415 Please take a look? I did not realize that syntax was not valid c++17 when reviewing #32935 ...

[mwswartwout]

I also didn't know that designated initializers were C++20, I thought they were C++17. Doing some googling it seems that most (all?) major compilers have added early support for them in C++17, and don't complain unless -Wpedantic or -Werror are enabled.

@ksperling-apple did you encounter a build environment that failed to compile with these? If we do remove these I think we should add something to CI that will break if they're added again in the future. I'm not sure the feasibility of enabling -Wpedantic but I'm guessing that would be very difficult and it sounds like there's no finer grained knob available to turn off designated initializers in clang.

[ksperling-apple]

We do compile with -Werror on various platforms in CI. One of the annoying aspects of this sort of thing is that it can be fine in our CI, but not when you compile in a slightly different context, e.g. with a different toolchain.

I don't recall running into any build errors over these specific initializers, but I think in general I don't think they make great API due to the restrictive way they can be used in C++ (compared to C), like initialization order having to match declaration order without skipping any fields. This means reordering members or adding members anywhere other than at the end suddenly becomes a source-breaking change.

A constructor (or a static factory method in case of something like DeviceEvent) makes for better, more readable API and also provides easy ways to evolve those APIs without source breaking changes.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Remove stale label or comment or this will be closed in 30 days.