Hi On Thu, Apr 19, 2018 at 8:44 AM, Markus Armbruster <arm...@redhat.com> wrote: > Marc-André Lureau <marcandre.lur...@redhat.com> writes: > >> Hi, >> >> This series aims to get rid of the distinction between QObject, that >> must use qobject_incref/qobject_decref and its various derived types >> that have to use QINCREF/QDECREF. Instead, replace it with >> qobject_ref/qobject_unref for all types. > > This is a lovely improvement.
That's a good sign. > However, I think the code can't quite decide whether to embrace or > eschew type casts between QObject and its subtypes. It uses both, for different reasons > On the one hand, we provide safe conversion macros QOBJECT() and > qobject_to(). By the way, shouting one but not the other is a bit ugly. QOBJECT is static upcast, the compiler will shout. qobject_to() is dynamic downcast, the runtime shout. > On the other hand, we still use type casts, and enforce "base comes > first" to make them work. > > If we want to eschew type casts, we should clean up the remaining ones. > The case for "base must come first" is quite weak then. Not worth the > bother, I think. > > I doubt we want to embrace type casts here. If we wanted to, I'd go all > the way and unwrap base in QObjects and its subtypes with suitable "must > match QObject" comments. Alternatively, use an unnamed member. Then > convert with type casts rather than container_of() and &->base. > Can you make a different patch? Can it be done after? Should we add a note there for future improvements? I don't like to spent too much time on this, yet as you said, this is a lovely improvement, so is there any real blocker left? -- Marc-André Lureau