Details

Reviewers

zzag

Group Reviewers

KWin

Commits

R108:cca0e15b455b: Fix compiler warnings

Summary

No need to keep them around for no reason.

Test Plan

Tested the plugins I thought could be affected. Have been using it for a couple of days without problems

Diff Detail

Repository

R108 KWin

Branch

master

Lint

Lint Errors

Excuse: yyy

Severity	Location	Code	Message
Error	composite.h:203	Cppcheck	unknownMacro
Error	input.h:310	Cppcheck	unknownMacro
Error	input.h:310	Cppcheck	unknownMacro
Error	input.h:310	Cppcheck	unknownMacro
Error	libkwineffects/kwinglobals.h:150	Cppcheck	unknownMacro
Error	utils.h:123	Cppcheck	unknownMacro
Error	utils.h:123	Cppcheck	unknownMacro
Error	utils.h:123	Cppcheck	unknownMacro

Unit

No Unit Test Coverage

Build Status

Buildable 23770
Build 23788: arc lint + arc unit

apol created this revision.Mar 15 2020, 8:00 PM

Restricted Application added a project: KWin. · View Herald TranscriptMar 15 2020, 8:00 PM

Restricted Application added a subscriber: kwin. · View Herald Transcript

Harbormaster completed remote builds in B23723: Diff 77679.Mar 15 2020, 8:00 PM

apol requested review of this revision.Mar 15 2020, 8:00 PM

zzag added a subscriber: zzag.Mar 16 2020, 7:32 AM

zzag added inline comments.

debug_console.cpp
713	According to Qt documentation, we should interpret QVariant::Type as QMetaType::Type. Although this function is declared as returning QVariant::Type, the return value should be interpreted as QMetaType::Type https://doc.qt.io/qt-5/qvariant.html#type
effects/screenshot/screenshot.cpp
661	Do compilers bark about using c-style casts?
main_wayland.cpp
264	Indentation is off.
plugins/kdecorations/aurorae/src/aurorae.cpp
599	Unrelated side note: this is really wrong!
wayland_server.cpp
734	Unrelated side note: I don't get why Qt folks deprecated toSet(). Range-based initialization looks very clunky if you ask me.

apol added inline comments.Mar 16 2020, 2:20 PM

effects/screenshot/screenshot.cpp
661	The warning is: `kwin/effects/screenshot/screenshot.cpp:671:23: warning: cast from 'uchar ' (aka 'unsigned char ') to 'uint ' (aka 'unsigned int ') increases required alignment from 1 to 4 [-Wcast-align]`

apol added inline comments.Mar 16 2020, 2:22 PM

wayland_server.cpp
734	I don't get it either.

Address comments by Vlad

Harbormaster completed remote builds in B23761: Diff 77734.Mar 16 2020, 2:22 PM

Perhaps it would be worth to split this patch because it fixes a lot of issues. Overall, +1.

effects/screenshot/screenshot.cpp
661	Coding style nitpick: add whitespace before `*`.
671	Looks like you forgot to add reinterpret_cast.

add space

Harbormaster completed remote builds in B23767: Diff 77742.Mar 16 2020, 3:29 PM

Properly use QProcess

Harbormaster completed remote builds in B23770: Diff 77752.Mar 16 2020, 5:21 PM

zzag added inline comments.Mar 17 2020, 7:56 AM

debug_console.cpp
713	Could you please revert this change? The documentation says that we should use QMetaType. I assume the compiler doesn't like conversions between QVariant::Type and QMetaType::Type. If that's the case, we could cast the return value of `value.type()` or use `value.userType()` as Qt folks do.

apol marked 2 inline comments as done.Mar 17 2020, 12:39 PM

apol added inline comments.

debug_console.cpp
713	It doesn't make much sense since QVartiant's are defined in terms of QMetaType: `SizeF = QMetaType::QSizeF`. But sure, changing for your peace of mind.

apol marked an inline comment as done.Mar 17 2020, 12:41 PM

Prefer using switch() with int to mute the warning

Harbormaster completed remote builds in B23811: Diff 77819.Mar 17 2020, 12:41 PM

zzag added inline comments.Mar 17 2020, 1:35 PM

effects/showfps/showfps.cpp
126–127	`wasRunning` is unused.
main_wayland.cpp
262–264	Could you please walk me through this change? Why do we need to call setProgram()?

apol marked an inline comment as done.Mar 17 2020, 1:44 PM

apol added inline comments.

main_wayland.cpp
262–264	`void start(const QString &command, OpenMode mode = ReadWrite);` is deprecated. We could also use `void start(const QString &program, const QStringList &arguments, OpenMode mode = ReadWrite);`

zzag added inline comments.Mar 17 2020, 1:45 PM

main_wayland.cpp
262–264	Is it deprecated in Qt 5.15?

zzag accepted this revision.Mar 17 2020, 2:00 PM

zzag added inline comments.

effects/showfps/showfps.cpp
126–127	Please address this comment before landing.
plugins/platforms/virtual/egl_gbm_backend.cpp
197	nit: add whitespace before `*`

This revision is now accepted and ready to land.Mar 17 2020, 2:00 PM

apol marked an inline comment as done.Mar 17 2020, 2:07 PM

Closed by commit R108:cca0e15b455b: Fix compiler warnings (authored by apol). · Explain WhyMar 17 2020, 2:08 PM

This revision was automatically updated to reflect the committed changes.

Hmm, it looks like CI still uses Qt 5.12. :(

https://build.kde.org/job/Plasma/job/kwin/job/kf5-qt5%20SUSEQt5.12/620/console

This change broke the show fps effect.

This change broke launching applications with arguments, e.g. dbus-run-session kwin_wayland --width 1920 --height 1080 --xwayland "kate file.txt"

			Path	Packages
M			CMakeLists.txt (2 lines)
M			abstract_wayland_output.cpp (2 lines)
M			debug_console.cpp (10 lines)
M			decorations/settings.cpp (4 lines)
M			effects/highlightwindow/highlightwindow.cpp (4 lines)
M			effects/screenshot/screenshot.cpp (4 lines)
M			effects/showfps/showfps.h (4 lines)
M			effects/showfps/showfps.cpp (9 lines)
M			input.cpp (2 lines)
M			kcmkwin/kwindecoration/declarative-plugin/previewclient.cpp (2 lines)
M			kcmkwin/kwintabbox/main.cpp (2 lines)
M			layers.cpp (4 lines)
M			libkwineffects/kwinglplatform.cpp (3 lines)
M			main_wayland.cpp (21 lines)
M			plugins/kdecorations/aurorae/src/aurorae.cpp (30 lines)
M			plugins/platforms/drm/drm_backend.cpp (8 lines)
M			plugins/platforms/drm/edid.cpp (2 lines)
M			plugins/platforms/fbdev/scene_qpainter_fb_backend.cpp (2 lines)
M			plugins/platforms/virtual/egl_gbm_backend.cpp (4 lines)
M			plugins/platforms/wayland/wayland_backend.cpp (6 lines)
M			plugins/platforms/x11/standalone/glxbackend.cpp (6 lines)
M			plugins/scenes/opengl/scene_opengl.cpp (2 lines)
M			tabbox/clientmodel.cpp (28 lines)
M			useractions.cpp (2 lines)
M			wayland_server.cpp (6 lines)
M			xwl/drag_x.cpp (2 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	77679	e8efa83		Mar 15 2020, 8:00 PM	★	★
Diff 2	77734	ede53f7	Address comments by Vlad	Mar 16 2020, 2:22 PM	★	★
Diff 3	77742	7c1c9d8	add space	Mar 16 2020, 3:29 PM	★	★
Diff 4	77752	7c1c9d8	Properly use QProcess	Mar 16 2020, 5:21 PM	★	★
Diff 5	77819	518c85e	Prefer using switch() with int to mute the warning	Mar 17 2020, 12:41 PM	★	★
Diff 6	77823	39154ad	R108:cca0e15b455ba606043c720479490794d8d86309	Mar 17 2020, 2:07 PM	★	★

Fix compiler warnings
ClosedPublic
Actions

Details

Diff Detail

Revision Contents
Changeset List

Diff 77752

CMakeLists.txt

abstract_wayland_output.cpp

debug_console.cpp

decorations/settings.cpp

effects/highlightwindow/highlightwindow.cpp

effects/screenshot/screenshot.cpp

effects/showfps/showfps.h

effects/showfps/showfps.cpp

input.cpp

kcmkwin/kwindecoration/declarative-plugin/previewclient.cpp

kcmkwin/kwintabbox/main.cpp

layers.cpp

libkwineffects/kwinglplatform.cpp

main_wayland.cpp

plugins/kdecorations/aurorae/src/aurorae.cpp

plugins/platforms/drm/drm_backend.cpp

plugins/platforms/drm/edid.cpp

plugins/platforms/fbdev/scene_qpainter_fb_backend.cpp

plugins/platforms/virtual/egl_gbm_backend.cpp

plugins/platforms/wayland/wayland_backend.cpp

plugins/platforms/x11/standalone/glxbackend.cpp

plugins/scenes/opengl/scene_opengl.cpp

tabbox/clientmodel.cpp

useractions.cpp

wayland_server.cpp

xwl/drag_x.cpp

Fix compiler warningsClosedPublicActions

Details

Diff Detail

Revision ContentsChangeset List

Diff 77752

CMakeLists.txt

abstract_wayland_output.cpp

debug_console.cpp

decorations/settings.cpp

effects/highlightwindow/highlightwindow.cpp

effects/screenshot/screenshot.cpp

effects/showfps/showfps.h

effects/showfps/showfps.cpp

input.cpp

kcmkwin/kwindecoration/declarative-plugin/previewclient.cpp

kcmkwin/kwintabbox/main.cpp

layers.cpp

libkwineffects/kwinglplatform.cpp

main_wayland.cpp

plugins/kdecorations/aurorae/src/aurorae.cpp

plugins/platforms/drm/drm_backend.cpp

plugins/platforms/drm/edid.cpp

plugins/platforms/fbdev/scene_qpainter_fb_backend.cpp

plugins/platforms/virtual/egl_gbm_backend.cpp

plugins/platforms/wayland/wayland_backend.cpp

plugins/platforms/x11/standalone/glxbackend.cpp

plugins/scenes/opengl/scene_opengl.cpp

tabbox/clientmodel.cpp

useractions.cpp

wayland_server.cpp

xwl/drag_x.cpp

Fix compiler warnings
ClosedPublic
Actions

Revision Contents
Changeset List