GCC 16 warnings about incomplete types in an SFINAE context

Christian Ehrlicher

std::bind? Use lambda :)

l3u_

Yeah, that would of course work as well. But as long as nothing changes – in this case, I'm only passing a pointer to a function call, always the same one – I think std::bind is more elegant? Or is there (meanwhile) a reason not to do something like (staying with the above example)

connect(spinBox, &BoogerSpinBox::scorePopupRequested,
        this, std::bind(&BoogerWidget::scorePopupRequested, this, spinBox));

?

JonB

@l3u_
We would write:

connect(spinBox, &BoogerSpinBox::scorePopupRequested,
        this, [this, spinBox]() { this->onScorePopupRequested(spinBox); } );

(The this-> just for clarity.) I don't know why you find std::bind() more "elegant" than this. You can also be more flexible on what you write in the lambda body than with a std::bind(), and may not even need an actual slot method at all. At any rate, you will see many examples using lambda-connect()s in Qt code.

l3u_

I actually do use lamdba connects in many places … well, maybe, it's a question of style and taste then.

However, I have to admit that the lambda version is a bit more clear about what's happening. Also, I'll never memorize that std::bind connect syntax, I have to look at another piece of my code each and every time I write one ;-)

SGaist

Out of curiosity, do you even then that signal in the first place ?
Can you explain what happens in the slot connected to it ?

l3u_

It's a quite simple use-case. I have a spinbox, whose value can also be set via a popup dialog. And the popup can be requested both through the context menu requested by the spinbox itself and through a radio button enabling the spinbox in the first place. Thus, the pointer to the spinbox that has to be updated is needed in the slot causing said popup to be shown, so that it's clear which spinbox's value has to be updated (there can be multiple ones).

Back in the day, I solved this by passing "this" in the signal emitted from the spinbox itself, or the respective pointer passed to the radio button enabling it – cf. the code above.

The original code has been written in "Effective Modern C++" C++11 times, so I sticked to std::bind when fixing the initial problem posted here. However, I now learned that std::bind was only needed when lambda connects weren't possible at all or not still reliable – but they are now. So there's no need to still use std::bind in this case.

Christian Ehrlicher

https://doc.qt.io/qt-6/qobject.html#sender

l3u_

Yeah, I know this – and I also read the big fat warnings saying "This function violates the object-oriented principle of modularity" ;-)

Also, I don't always need the sender, because the popup can be requested from a spinbox and from a radio button – but I always need the spinbox. So this couldn't even be used in this case.

SGaist

@l3u_ said in GCC 16 warnings about incomplete types in an SFINAE context:

Yeah, I know this – and I also read the big fat warnings saying "This function violates the object-oriented principle of modularity" ;-)

Also, I don't always need the sender, because the popup can be requested from a spinbox and from a radio button – but I always need the spinbox. So this couldn't even be used in this case.

Technically, you're doing the same violation since your dialog has to know which object has called it.

Usually, to keep things separated, it boils down to these two options:

Call the dialog in a fashion where you get back the value you want and set it to whatever control makes sense (e.g. QInputDialog)
Connect the dialog to the widget through signals and slots

If you are reusing the dialog, nothing forbids you to disconnect and reconnect your widgets again and again.

Plastic.Jesus

This bit my whole organization after we upgraded to Fedora 44 last week and got GCC 16 along for the ride.
Every single TU where we were forward declaring any class involved in a signal or slot declaration (rather than including the header directly) generates this warning. Since we run -Wall and -Werror as policy it basically borked our entire code base.

IMO this is really a Qt issue since the SFINAE-on-potentially-incomplete-type trait check is in moc's autogenerated output
and QtPrivate::QMetaTypeForType::check() as opposed to user code.
Qt's documentation has long encouraged forward-declaration in QObject headers for compile-time hygiene (rightly so!) That guidance is now actively unsafe under any -Werror build on GCC 14+.

Now I'm left with a choice. I either suppress the warning (which I absolutely hate), or I include the headers and take the coimpile time hit on a project with 250,000 lines of code.

It would be great if this could get fixed in the moc generation.

JonB

@Plastic.Jesus said in GCC 16 warnings about incomplete types in an SFINAE context:

It would be great if this could get fixed in the moc generation.

I am not affected by this issue, but understand what you are saying. But if you wish or hope for some change in moc generation you would need to open an issue at https://qt-project.atlassian.net/issues/

Plastic.Jesus

@JonB understood. Just trying to get my arms fully wrapped around the implications.

It turned out the compile-time hit was not as bad as I'd anticipated ~1-2% increase.

I think the biggest impact in the real world is going to be the ambiguity of the compile warnings (or errors if -Werror). The GCC output is only for the .moc and there is really no indication of any kind as to the actual offending header, as I believe is the genesis of this topic. The OP is digging around trying to figure out what's wrong with his class, but the actual issue is in some other QObject which uses the class in a signal or slot declaration.

Once I mess around with this a little more it'll probably be worth filing a ticket.

Thanks!

SGaist

@Plastic.Jesus I think that including the moc generated file at the bottom of your class implementation should also fix that. Can you check ?

Plastic.Jesus

@SGaist That is a really interesting approach that I never considered. I'm going to definitely try it out - always looking to shave seconds off compile time and this could really be a big win.
We've already added the appropriate #include's through our code base to clear up the SFINAE warnings, but I'm going to run a spike to see what a full-scale test of this approach buys us.
Thanks for the tip!

jeremy_k

To add support for @SGaist's suggestion, #including the moc output is standard procedure for Qt autotests, and has been for a very long time. There's quite a bit of it in the libraries as well.

SGaist

To add to @jeremy_k KDE folks also implements that technique in their libraries/software.