Headers use 'slots' keyword which conflict with QT code.

[marijnfs]

🐛 Bug

Many projects use QT for UI, but when compiling such code a special keyword "slots" is defined, which conflicts with the 'slots' function defined in IValue. Since IValue.h ends up being included when including Torch headers, this prevents using Torch in any code that also does QT stuff.

Even though this is not the fault of Torch, since QT is a popular cross-platform GUI library, it would probably be great for usability if that function is somehow renamed or some other smart trick is used.

To Reproduce

Steps to reproduce the behavior:

Tested with current Torch master.

1. Include some Torch headers in a file that uses QT slots functionality.

[suo]

I think the workaround here is to compile QT with QT_NO_KEYWORDS, which should change QT's slots to Q_SLOTS. Does that work for you?

To be honest, I'm reluctant to change JIT code just because QT has bad macro hygiene. It seems quite difficult to maintain (they have also reserved foreach, signals, and emit). So if QT_NO_KEYWORDS can be our blessed solution that would be ideal.

[xsacha]

Seems like a bad workaround as generally Qt code is written to use slots rather than Q_SLOTS.
This workaround would require recompiling Qt and doing a text replace on most Qt files.

[soumith]

it is sad that QT resorted to this, and it's too much of a burden on userland to understand and regex.
Let's change slots, as burden on userland is not great.

[ssnl]

@xsacha Do you really need to recompile Qt though? For boost, they have a similar problem, yet in their documentation, they say:

When building with Qt, the Moc keywords signals and slots are defined using preprocessor macros, causing programs using Boost.Signals and Qt together to fail to compile.

For Qt 4.1 and later, This behavior can be turned off in Qt on a per-project or per-file basis with the no_keywords option. This works with out-of-the-box builds of Boost and Qt. You do not need to re-configure, re-build, or duplicate existing libraries. For a project where you want to use both Boost.Signals and Qt Signals and Slots, the relevant part of your .pro file might look like this:

CONFIG      += no_keywords # so Qt won't #define any non-all-caps `keywords'
INCLUDEPATH += . /usr/local/include/boost-1_33_1/
macx:LIBS   += /usr/local/lib/libboost_signals-1_33_1.a  # ...your exact paths may vary

Now you can mix Boost.Signals and Qt Signals and Slots in the same files, and even within the same class or function. You will have to use the upper-case versions of Qt macros in your own code. See the article A Deeper Look at Signals and Slots [off-site] for more complete examples and a survey of the strengths of the two systems.

source: https://www.boost.org/doc/libs/1_65_1/doc/html/signals/s04.html#idp523098912

Also in Qt, CONFIG+=no_keywords has been a thing since at least 3 years ago:
https://forum.qt.io/topic/72265/purpose-of-q_slots-q_signals-q_emit
Their doc seems to also suggestion that this isn't that complicated:
https://doc.qt.io/qt-5/signalsandslots.html#using-qt-with-3rd-party-signals-and-slots

[xsacha]

Ah I see. So it wouldn't need a recompile but still needs a text replacement. Not so bad.

[suo]

@xsacha if you don't want to do the text replacement, something like:

#undef slots
#include "torch/torch.h"
#def slots Q_SLOTS

is apparently what the QT documentation recommends.

I'll put up a PR to remove slot and slots from ivalue.h but it will be hard to guard regressions like this in the future, so the above might be more robust.

[marijnfs]

That's a pretty good work around I guess, I'll try it out.

[suo]

Okay, I'm going to close this out for now since it seems like we have a decent workaround. If others face this issue/feel strongly like the workaround is bad, feel free to reopen and we can discuss.