diff options
author | Marc Mutz <marc.mutz@qt.io> | 2022-02-06 22:19:27 +0100 |
---|---|---|
committer | Marc Mutz <marc.mutz@qt.io> | 2022-03-11 19:52:40 +0000 |
commit | 48d04fbbbf66d6f503c4bbcffab63cb420408b05 (patch) | |
tree | 6df77309c24b2058075b5cddaa3090ffbb5bebc4 /tests | |
parent | a9c5dd09215e5806a57b01c7c4fab366860f15f2 (diff) | |
download | qtbase-48d04fbbbf66d6f503c4bbcffab63cb420408b05.tar.gz |
Fix C++20 ambiguous relational operators between QJsonValue{,Ref}
In C++20, any given relational operator is also considered in its
reversed form, so e.g.
given op==(X, Y)
and X x, Y y, then y == x will compile, by using the reversed op(X, Y)
This, unfortunately, makes some existing asymmetric operator overload
sets ambiguous, and instead of applying tie-breaker rules, at least
Clang is warning about these.
For us, this means we need to make our overload set non-ambiguous. The
QJsonValue{,Ref} classes failed this, because they only provide the
following member-operators:
- QJsonValue::op==(const QJsonValue&) const
- QJsonValueRef::op==(const QJsonValue &) const
For member functions, there are no implicit conversions on the LHS. So
in C++17, we have a nice dichotomous overload set:
- LHS is QJsonValue -> use QJsonValue::op==(QJsonValue)
- LHS is QJsonValueRef -> use QJsonValueRef::op==(QJsonValue)
In both of these, it the RHS is a QJsonValueRef, it's implicitly
converted to QJsonValue for the call.
Enter C++20, and the reversed signatures are suddenly available, too,
which is a problem for QJsonValueRef <> QJsonValueRef, which could be
resolved, as in C++17, using
lhs.QJVR::op==(QJV(rhs))
or it could now be
rhs.QJVR::op==(QJV(lhs)); // reversed
Says Clang 10:
tst_qtjson.cpp:990:5: warning: ISO C++20 considers use of overloaded operator '==' (with operand types 'QJsonValueRef' and 'QJsonValueRef') to be ambiguous despite there being a unique best viable function [-Wambiguous-reversed-operator]
CHECK(r0, a0, r1);
^ ~~ ~~
qjsonvalue.h:189:17: note: ambiguity is between a regular call to this operator and a call with the argument order reversed
inline bool operator==(const QJsonValue &other) const { return toValue() == other; }
^
A similar argument makes op!= ambiguous.
Says Clang 10:
tst_qtjson.cpp:988:5: error: use of overloaded operator '!=' is ambiguous (with operand types 'QJsonValueRef' and 'QJsonValueRef')
CHECK(r0, r0, r1);
^ ~~ ~~
qjsonvalue.h:190:17: note: candidate function
inline bool operator!=(const QJsonValue &other) const { return toValue() != other; }
^
qjsonvalue.h:189:17: note: candidate function
inline bool operator==(const QJsonValue &other) const { return toValue() == other; }
^
qjsonvalue.h:189:17: note: candidate function (with reversed parameter order)
To fix, provide the missing operators as free inline functions (so Qt
6.2 and 5.15 don't get new symbols added) so there's always exactly
one best match.
This is a fix for 6.2 and 5.15. At the time of writing, 6.3 isn't
released, yet, so there, we could QT_REMOVED_SINCE the pre-existing
member operators in favor of hidden friends (as per QTBUG-87973).
Use C++17'isms to prevent an automatic merge to 5.15, which requires
contains(QT_CONFIG,c++2a):CONFIG += c++2a
added to tst_qtjson.pro.
[ChangeLog][QtCore][QJsonValue] Fixed relational operators to not
cause warnings/ambiguities when compiling in C++20.
Change-Id: Ic70f3cad9987c87f7346d426c29cc2079d85ad13
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Reviewed-by: Lars Knoll <lars.knoll@qt.io>
(cherry picked from commit ec03db7bb394c6d3e1b77e1532d26551da1dde78)
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Diffstat (limited to 'tests')
-rw-r--r-- | tests/auto/corelib/serialization/json/json.pro | 1 | ||||
-rw-r--r-- | tests/auto/corelib/serialization/json/tst_qtjson.cpp | 52 |
2 files changed, 53 insertions, 0 deletions
diff --git a/tests/auto/corelib/serialization/json/json.pro b/tests/auto/corelib/serialization/json/json.pro index 8fa17c5c38..390a674f7a 100644 --- a/tests/auto/corelib/serialization/json/json.pro +++ b/tests/auto/corelib/serialization/json/json.pro @@ -1,6 +1,7 @@ TARGET = tst_json QT = core-private testlib CONFIG += testcase +contains(QT_CONFIG, c++2a):CONFIG *= c++2a !android:TESTDATA += bom.json test.json test.bjson test3.json test2.json else:RESOURCES += json.qrc diff --git a/tests/auto/corelib/serialization/json/tst_qtjson.cpp b/tests/auto/corelib/serialization/json/tst_qtjson.cpp index c8f82ef5d5..3f242ba2a0 100644 --- a/tests/auto/corelib/serialization/json/tst_qtjson.cpp +++ b/tests/auto/corelib/serialization/json/tst_qtjson.cpp @@ -68,6 +68,7 @@ private Q_SLOTS: void testObjectNestedEmpty(); void testValueRef(); + void testValueRefComparison(); void testObjectIteration(); void testArrayIteration(); @@ -910,6 +911,57 @@ void tst_QtJson::testValueRef() QCOMPARE(object.value(QLatin1String("key")), QJsonValue(42)); } +void tst_QtJson::testValueRefComparison() +{ + QJsonValue a0 = 42.; + QJsonValue a1 = QStringLiteral("142"); + +#define CHECK_IMPL(lhs, rhs, ineq) \ + QCOMPARE(lhs, rhs); \ + QVERIFY(!(lhs != rhs)); \ + QVERIFY(lhs != ineq); \ + QVERIFY(!(lhs == ineq)); \ + QVERIFY(ineq != rhs); \ + QVERIFY(!(ineq == rhs)); \ + /* end */ + +#define CHECK(lhs, rhs, ineq) \ + do { \ + CHECK_IMPL(lhs, rhs, ineq) \ + CHECK_IMPL(qAsConst(lhs), rhs, ineq) \ + CHECK_IMPL(lhs, qAsConst(rhs), ineq) \ + CHECK_IMPL(qAsConst(lhs), qAsConst(rhs), ineq) \ + } while (0) + + // check that the (in)equality operators aren't ambiguous in C++20: + QJsonArray a = {a0, a1}; + + Q_STATIC_ASSERT((std::is_same<decltype(a[0]), QJsonValueRef>::value)); + + auto r0 = a.begin()[0]; + auto r1 = a.begin()[1]; + auto c0 = qAsConst(a).begin()[0]; + // ref <> ref + CHECK(r0, r0, r1); + // cref <> ref + CHECK(c0, r0, r1); + // ref <> cref + CHECK(r0, c0, r1); + // ref <> val + CHECK(r0, a0, r1); + // cref <> val + CHECK(c0, a0, r1); + // val <> ref + CHECK(a0, r0, a1); + // val <> cref + CHECK(a0, c0, a1); + // val <> val + CHECK(a0, a0, a1); + +#undef CHECK +#undef CHECK_IMPL +} + void tst_QtJson::testObjectIteration() { QJsonObject object; |