Fix undefined behavior and set QT_MIN_VERSION to 5.9.0
ClosedPublic

Authored by mglb on Mar 18 2019, 3:54 PM.

Details

Summary

Arguments in some QCOMPARE calls in KeyboardTranslatorTest had slightly
different types, what according to Qt documentation is not allowed.
The types of both QCOMPARE arguments must be exactly the same, otherwise
the code doesn't compile with some Qt versions.

With this fix it is safe to lower minimum Qt version to 5.9.0. Patch
number is incremented only for bugfix releases, so as long as invalid
code is not used everything should work.

The fix allows to compile Konsole using system libs on larger number
of systems, e.g. Ubuntu 18.04 LTS (Qt 5.9.5).

Tested with Qt 5.9.[0-5] and 5.11.1

Diff Detail

Repository
R319 Konsole
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mglb requested review of this revision.Mar 18 2019, 3:54 PM
mglb created this revision.

It fails here w/ 5.9.4 in the tests and .7 was the current so that's why I picked it.

To get it compile w/ 5.9.4, I have to comment out all the QCOMPAREs - I thought I tested this w/ 5.9.7 but I appear to have upgraded my VMs

src/autotests/KeyboardTranslatorTest.cpp:112: error: undefined reference to 'bool QTest::qCompare<Qt::Key, int>(Qt::Key const&, int const&, char const*, char const*, char const*, int)'

I changed it to 5.9.4; it required some #ifs in KeyboardTranslatorTest.cpp; the only 5.9.x I could find were .4 and .5

mglb updated this revision to Diff 54461.Mar 21 2019, 2:19 AM
  • Fix argument types in QCOMPARE (fixes builds)
  • Update description
mglb added a comment.Mar 21 2019, 2:19 AM

To get it compile w/ 5.9.4, I have to comment out all the QCOMPAREs - I thought I tested this w/ 5.9.7 but I appear to have upgraded my VMs

src/autotests/KeyboardTranslatorTest.cpp:112: error: undefined reference to 'bool QTest::qCompare<Qt::Key, int>(Qt::Key const&, int const&, char const*, char const*, char const*, int)'

Turns out it was a code with undefined behavior:

QCOMPARE is very strict on the data types. Both actual and expected have to be of the same type, otherwise the test won't compile. This prohibits unspecified behavior from being introduced; that is behavior that usually occurs when the compiler implicitly casts the argument.
https://doc.qt.io/qt-5.9/qtest.html#QCOMPARE

I changed it to 5.9.4; it required some #ifs in KeyboardTranslatorTest.cpp; the only 5.9.x I could find were .4 and .5

When I need different Qt version than available in OS repos, I use prebuilt Qt from qt.io - all Qt5 versions available (at least in online installer).

mglb updated this revision to Diff 54462.Mar 21 2019, 2:21 AM
mglb retitled this revision from Set patch number in QT_MIN_VERSION to 0 to Fix undefined behavior and set QT_MIN_VERSION to 5.9.0.
mglb edited the summary of this revision. (Show Details)

Update description

OK, thanks - I thought about casting but it seemed strange to only affect Qt 5.9.x

hindenburg accepted this revision.Mar 21 2019, 2:41 AM
This revision is now accepted and ready to land.Mar 21 2019, 2:41 AM
This revision was automatically updated to reflect the committed changes.