Fix GCC8 warnings
ClosedPublic

Authored by orgads on Nov 8 2018, 6:10 PM.

Details

Diff Detail

Repository
R49 KCacheGrind
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
orgads requested review of this revision.Nov 8 2018, 6:10 PM
orgads created this revision.

@weidendo i guess this is for you :)

yurchor added a subscriber: yurchor.Nov 9 2018, 8:59 AM
yurchor added inline comments.
libcore/cachegrindloader.cpp
1087

If it worked earlier, shouldn't it be

Q_FALLTHROUGH();

?

libcore/tracedata.cpp
1908

Same here.

orgads added inline comments.Nov 9 2018, 9:01 AM
libcore/cachegrindloader.cpp
1087

Is there an option "scalls"? And is it equivalent to "rcalls"?

And Q_FALLTHROUGH requires Qt5. From the README it looks like you support Qt4 as well...

libcore/tracedata.cpp
1908

See above.

yurchor added inline comments.Nov 9 2018, 9:50 AM
libcore/cachegrindloader.cpp
1087

H-m-m... Can you provide a recipe to build the current git/master with Qt 4?

In CMakeLists.txt,

set (QT_MIN_VERSION "5.2.0")

https://cgit.kde.org/kcachegrind.git/tree/CMakeLists.txt#n6

In qcg.pro,

lessThan(QT_MAJOR_VERSION, 5) {
    error("QCachegrind requires Qt 5.3 or greater")
}

https://cgit.kde.org/kcachegrind.git/tree/qcg.pro#n6

It seems to me that it is only possible to build with Qt 5 these days.

orgads added inline comments.Nov 9 2018, 9:59 AM
libcore/cachegrindloader.cpp
1087

Then you should update the README :)

Anyway, Q_FALLTHROUGH was introduced in Qt 5.8, so you still can't use it.

http://doc.qt.io/qt-5/qtglobal.html#Q_FALLTHROUGH

aacid added inline comments.Nov 9 2018, 7:35 PM
libcore/cachegrindloader.cpp
1087

A proper comment like
// fall through
is fine, gcc "understands" it.

aacid accepted this revision.Nov 28 2018, 11:21 PM

Ok @weidendo didn't want to have a look, i'll approve and commit it myself then

This revision is now accepted and ready to land.Nov 28 2018, 11:21 PM
Closed by commit R49:158d89b44eb3: Fix GCC8 warnings (authored by orgads, committed by aacid). · Explain WhyNov 28 2018, 11:21 PM
This revision was automatically updated to reflect the committed changes.

Sorry for my late reaction.

Yes, current master only supports Qt5; Yuri, thanks for updating the README!

Adding the "break" is correct. It just did never triggered as summary lines do not
contain the text "calls=".

And good to know that GCC catches "fall through" comments, if correctly written.