On 18 Mar 2024, at 14:00, Giuseppe D'Angelo via Development <development@qt-project.org> wrote:
On 18/03/2024 13:34, André Somers wrote: While I know it's easy to work around, I sometimes find myself doing it anyway. To me, it signals what API is intended to be used in what way. That a class overrides `event` (or any other public virtual method) does not mean that that method is then intended to be called by a user of the class as the type you defined. That you overwrote it may just be an implementation detail. I think the methods you expose as "public" on an API are quite important*. They signal how the user is supposed to use an instance of your class. If you have methods in there that are just implementation details, then those don't fit. These methods are meant to be called by parts of the system that don't see your type as the actual type, but as something more basic: a QObject in this case. But I agree 100% here; this is typically realized in C++ by having the entry point public and non-virtual, and have that dispatch to a protected virtual. QObject::event() has been public since Qt 2.3 at least: https://doc.qt.io/archives/2.3/qobject.html And subclasses, by and large, always had those overrides as protected, e.g. https://doc.qt.io/archives/2.3/qwidget.html#6ff658 Was it because the old Trolls didn’t know better, or thought it would be a clever way to allow some specific use cases while preventing silly mistakes? The non-virtual interface pattern might have been around then already, and we do have a public entry point to QObject::event: QCoreApplication::send/postEvent, both of them linked to from the documentation. The function is also today still documented saying that it “receives events to an object"; the documentation doesn’t say “call this function if you want something to happen to the object”. Ironically, the QObject subclass in the snippet used also overrides event() as a public function (while the next snippet overrides eventFilter as a protected function). The whole problem we're discussing is that `event()` has been made public in the base class and that means it's now public API of any QObject subclass, whether they like it or not. :-( People can call QObject::event, as long as they cast to, or store their pointer as, a QObject, and as long as they know how to construct and correctly manage the life time of the respective QEvent subclass instance (unless they merely forward an existing one). I think that’s fine. Folks have evidently managed to do so for the last two decades, and the amount of foot-shooting seems to have been manageable. Perhaps it’s ok to expect that people that jump through those kinds of hoops read https://doc.qt.io/qt-6/eventsandfilters.html and know what they are doing. As I see it, nothing we can do to change this (make QObject::event protected, add a public NVI function) would improve Qt in a way that would justify the work. I’ll happily review a change to the documentation that fixes the snippet, and adds a sentence about using QCoreApplication::send- or postEvent instead, perhaps linking explicitly to https://doc.qt.io/qt-6/eventsandfilters.html. Maybe at some point the C++ committee will decide that compilers should warn if an override is not at least as strict as the virtual declaration. I’ll stand corrected then. Volker
-- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
