From a7da8e0dab8f6290b2af9bc12c0cd391af30bfb7 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Thu, 22 Jul 2021 19:13:27 +0200 Subject: QWidget: cope with QObject::connect()'s incomplete SFINAE-friendliness The plan for QObject::connect() (perfect) forwarders, such as QWidget::addAction(), was to just use a variant of the Detection Idiom to see whether QObject::connect() with the arguments as given would compile and SFINAE out the forwarder otherwise. It turns out that the "functor" overload of QObject::connect(), in particular, is severly underconstrained and accepts e.g. QKeySequence as a function object, only erroring out via a static_assert() in the body of the function, and thus at instantiation time and not, as needed, at overload resolution time. At the same time, we don't really want QObject::connect() to SFINAE out on argument mismatches between signal and slot, because the resulting error messages would be ... unkind to users of the API. We would like to keep the static_assert()s for easier error reporting. Reconciling these two contradicting requirements has so far eluded this author, so for now, to unblock progress, we explicitly black- and, in one case, white-list possible arguments. Because QKeySequence, in particular, is implicitly constructible from int(!), and therefore any enum type(!), incl. Qt::ConnectionType, we need to do way too much coding in the addAction() constraints. Hopefully, we'll be able to fix the issue at the root cause, in QObject, before Qt 6.3 is out, but until then, this is an ok-ish stop-gap measure. Add thorough overload set checks (positive ones only, for now) to tst_qwidget and tst_qmenu. Change-Id: Ia05233df818bc82ecc924fc44c1b349af41cbbf1 Reviewed-by: Volker Hilsheimer --- tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp | 67 +++++++++++++++++++++++ 1 file changed, 67 insertions(+) (limited to 'tests/auto/widgets/kernel') diff --git a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp index 7ca08993ab..4fa568228a 100644 --- a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp +++ b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp @@ -160,6 +160,7 @@ public slots: void initTestCase(); void cleanup(); private slots: + void addActionOverloads(); void getSetCheck(); void fontPropagation(); void fontPropagation2(); @@ -644,6 +645,72 @@ void tst_QWidget::cleanup() QTRY_VERIFY(QApplication::topLevelWidgets().isEmpty()); } +template +struct ImplicitlyConvertibleTo { + T t; + operator const T() const { return t; } + operator T() { return t; } +}; + +void testFunction0() {} +void testFunction1(bool) {} + +void tst_QWidget::addActionOverloads() +{ + // almost exhaustive check of addAction() overloads: + // (text), (icon, text), (icon, text, shortcut), (text, shortcut) + // each with a good sample of ways to QObject::connect() to + // QAction::triggered(bool) + QWidget w; + + // don't just pass QString etc - that'd be too easy (think QStringBuilder) + ImplicitlyConvertibleTo text = {QStringLiteral("foo")}; + ImplicitlyConvertibleTo icon; + + const auto check = [&](auto &...args) { // don't need to perfectly-forward, only lvalues passed + w.addAction(args...); + + w.addAction(args..., &w, SLOT(deleteLater())); + w.addAction(args..., &w, &QObject::deleteLater); + w.addAction(args..., testFunction0); + w.addAction(args..., &w, testFunction0); + w.addAction(args..., testFunction1); + w.addAction(args..., &w, testFunction1); + w.addAction(args..., [&](bool b) { w.setEnabled(b); }); + w.addAction(args..., &w, [&](bool b) { w.setEnabled(b); }); + + w.addAction(args..., &w, SLOT(deleteLater()), Qt::QueuedConnection); + w.addAction(args..., &w, &QObject::deleteLater, Qt::QueuedConnection); + // doesn't exist: w.addAction(args..., testFunction0, Qt::QueuedConnection); + w.addAction(args..., &w, testFunction0, Qt::QueuedConnection); + // doesn't exist: w.addAction(args..., testFunction1, Qt::QueuedConnection); + w.addAction(args..., &w, testFunction1, Qt::QueuedConnection); + // doesn't exist: w.addAction(args..., [&](bool b) { w.setEnabled(b); }, Qt::QueuedConnection); + w.addAction(args..., &w, [&](bool b) { w.setEnabled(b); }, Qt::QueuedConnection); + }; + const auto check1 = [&](auto &arg, auto &...args) { + check(arg, args...); + check(std::as_const(arg), args...); + }; + const auto check2 = [&](auto &arg1, auto &arg2, auto &...args) { + check1(arg1, arg2, args...); + check1(arg1, std::as_const(arg2), args...); + }; + [[maybe_unused]] + const auto check3 = [&](auto &arg1, auto &arg2, auto &arg3) { + check2(arg1, arg2, arg3); + check2(arg1, arg2, std::as_const(arg3)); + }; + + check1(text); + check2(icon, text); +#ifndef QT_NO_SHORTCUT + ImplicitlyConvertibleTo keySequence = {Qt::CTRL | Qt::Key_C}; + check2(text, keySequence); + check3(icon, text, keySequence); +#endif +} + void tst_QWidget::fontPropagation() { QScopedPointer testWidget(new QWidget); -- cgit v1.2.3