Use dynamic_cast for downcasting instead of static_cast. #12325

Similar to #10498 , abide by the CppCodeGuidelines to enforce type safe casts.

See: https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c146-use-dynamic_cast-where-class-hierarchy-navigation-is-unavoidable

There's a sizable performance penalty in using dynamic_cast instead of static_cast due to the extra checks, and it needs RTTI type information enabled. I think it's unnecessary in all of these cases as the type of the object is already checked through qt's own introspection system. There's no need to do it through C++'s too.

Doubt there would be any performance issues in this particular case.

Since Qt is a 3rd party library I wouldn't rely on static_casting it.

If the code must compile with RTTI disabled I believe the Qt solution is qobject_cast https://doc.qt.io/qt-5/qobject.html#qobject_cast

So do you see any concrete problems with the current code? Any realistic way this can fail the way it is now?

If not, I don't see a reason to change it.

We don't use dynamic_cast anywhere in the code and without a very good reason we shouldn't introduce it either.

Unless something happens inside Qt, the code will function correctly.

I just have strong feeling about stuff like this.

The qt doc examples use static_cast -- http://doc.qt.io/qt-5/eventsandfilters.html so this seems fine as is, and even with these changes mistakes would still cause a crash since a nullptr returned by dynamic_cast will be immediately dereferenced. Mightn't be terrible to use dynamic_cast (or qobject_cast) in the if() statement instead of checking event->type(), but not sure it would be worth the effort.

NACK. See ☝️ by @ajtowns.

Closing, there seems to be agreement to not do this