whytofly
/
qt6-bb10
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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 <volker.hilsheimer@qt.io>
bb10
Marc Mutz 2021-07-22 19:13:27 +02:00
parent bd594f9457
commit a7da8e0dab
3 changed files with 150 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
src/widgets/kernel/qwidget.h
View File

@ -211,11 +211,28 @@ class Q_WIDGETS_EXPORT QWidget : public QObject, public QPaintDevice
Q_PROPERTY(QString windowFilePath READ windowFilePath WRITE setWindowFilePath)
Q_PROPERTY(Qt::InputMethodHints inputMethodHints READ inputMethodHints WRITE setInputMethodHints)
#if 0
// ### TODO: make this work (requires SFINAE-friendly connect())
template <typename...Args>
using compatible_action_slot_args = std::void_t<
decltype(QObject::connect(std::declval<QAction*>(), &QAction::triggered,
std::declval<Args>()...))
>;
#else
// good-enough compromise for now
template <typename...Args>
using compatible_action_slot_args = std::enable_if_t<std::conjunction_v<
#if QT_CONFIG(shortcut)
std::disjunction<
std::is_same<Args, Qt::ConnectionType>,
std::negation<std::is_convertible<Args, QKeySequence>>
>...,
#endif
std::negation<std::is_convertible<Args, QIcon>>...,
std::negation<std::is_convertible<Args, const char*>>...,
std::negation<std::is_convertible<Args, QString>>...
>>;
#endif
public:
enum RenderFlag {
DrawWindowBackground = 0x1,

67
tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp
View File

@ -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 <typename T>
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<QString> text = {QStringLiteral("foo")};
ImplicitlyConvertibleTo<QIcon> 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<QKeySequence> keySequence = {Qt::CTRL | Qt::Key_C};
check2(text, keySequence);
check3(icon, text, keySequence);
#endif
}
void tst_QWidget::fontPropagation()
{
QScopedPointer<QWidget> testWidget(new QWidget);

90
tests/auto/widgets/widgets/qmenu/tst_qmenu.cpp
View File

@ -287,44 +287,86 @@ void tst_QMenu::addActionsAndClear()
QCOMPARE(menus[0]->actions().count(), 0);
}
static void testFunction() { }
static void testFunction0() {}
static void testFunction1(bool) {}
template <typename T>
struct ImplicitlyConvertibleTo {
T t;
operator const T() const { return t; }
operator T() { return t; }
};
void tst_QMenu::addActionsConnect()
{
// 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)
QMenu menu;
const QString text = QLatin1String("bla");
const QIcon icon;
menu.addAction(text, &menu, SLOT(deleteLater()));
menu.addAction(text, &menu, &QMenu::deleteLater);
menu.addAction(text, testFunction);
menu.addAction(text, &menu, testFunction);
menu.addAction(icon, text, &menu, SLOT(deleteLater()));
menu.addAction(icon, text, &menu, &QMenu::deleteLater);
menu.addAction(icon, text, testFunction);
menu.addAction(icon, text, &menu, testFunction);
// don't just pass QString etc - that'd be too easy (think QStringBuilder)
ImplicitlyConvertibleTo<QString> text = {QLatin1String("bla")};
ImplicitlyConvertibleTo<QIcon> icon;
const auto check = [&](auto &...args) { // don't need to perfectly-forward, only lvalues passed
menu.addAction(args...);
menu.addAction(args..., &menu, SLOT(deleteLater()));
menu.addAction(args..., &menu, &QMenu::deleteLater);
menu.addAction(args..., testFunction0);
menu.addAction(args..., &menu, testFunction0);
menu.addAction(args..., testFunction1);
menu.addAction(args..., &menu, testFunction1);
menu.addAction(args..., [&](bool b) { menu.setEnabled(b); });
menu.addAction(args..., &menu, [&](bool b) { menu.setEnabled(b); });
menu.addAction(args..., &menu, SLOT(deleteLater()), Qt::QueuedConnection);
menu.addAction(args..., &menu, &QMenu::deleteLater, Qt::QueuedConnection);
// doesn't exist: menu.addAction(args..., testFunction0, Qt::QueuedConnection);
menu.addAction(args..., &menu, testFunction0, Qt::QueuedConnection);
// doesn't exist: menu.addAction(args..., testFunction1, Qt::QueuedConnection);
menu.addAction(args..., &menu, testFunction1, Qt::QueuedConnection);
// doesn't exist: menu.addAction(args..., [&](bool b) { menu.setEnabled(b); }, Qt::QueuedConnection);
menu.addAction(args..., &menu, [&](bool b) { menu.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
const QKeySequence keySequence(Qt::CTRL | Qt::Key_C);
ImplicitlyConvertibleTo<QKeySequence> keySequence = {Qt::CTRL | Qt::Key_C};
check2(text, keySequence);
check3(icon, text, keySequence);
#if QT_DEPRECATED_SINCE(6, 4)
QT_WARNING_PUSH
QT_WARNING_DISABLE_DEPRECATED
menu.addAction(text, &menu, SLOT(deleteLater()), keySequence);
menu.addAction(text, &menu, &QMenu::deleteLater, keySequence);
menu.addAction(text, testFunction, keySequence);
menu.addAction(text, &menu, testFunction, keySequence);
menu.addAction(text, testFunction0, keySequence);
menu.addAction(text, &menu, testFunction0, keySequence);
menu.addAction(text, testFunction1, keySequence);
menu.addAction(text, &menu, testFunction1, keySequence);
menu.addAction(icon, text, &menu, SLOT(deleteLater()), keySequence);
menu.addAction(icon, text, &menu, &QMenu::deleteLater, keySequence);
menu.addAction(icon, text, testFunction, keySequence);
menu.addAction(icon, text, &menu, testFunction, keySequence);
menu.addAction(icon, text, testFunction0, keySequence);
menu.addAction(icon, text, &menu, testFunction0, keySequence);
menu.addAction(icon, text, testFunction1, keySequence);
menu.addAction(icon, text, &menu, testFunction1, keySequence);
QT_WARNING_POP
#endif
menu.addAction(text, keySequence, &menu, SLOT(deleteLater()));
menu.addAction(text, keySequence, &menu, &QMenu::deleteLater);
menu.addAction(text, keySequence, testFunction);
menu.addAction(text, keySequence, &menu, testFunction);
menu.addAction(icon, text, keySequence, &menu, SLOT(deleteLater()));
menu.addAction(icon, text, keySequence, &menu, &QMenu::deleteLater);
menu.addAction(icon, text, keySequence, testFunction);
menu.addAction(icon, text, keySequence, &menu, testFunction);
#endif // !QT_NO_SHORTCUT
}