winft / disman

Qt/C++ display management library
GNU Lesser General Public License v2.1
2 stars 1 forks source link

5.20 Beta release fails to compile when when "CMAKE_BUILD_TYPE" is set to "Release" #25

Closed romangg closed 7 months ago

romangg commented 3 years ago

In GitLab by @Valmar on Sep 26, 2020, 07:25

FAILED: backends/wayland/plugins/kwayland/CMakeFiles/kwayland.dir/kwayland_output.cpp.o 
/run/media/valmar/Data/Packages/AUR/Tk-Glitch/mostlyportable-gcc/gcc-mostlyportable-current/bin/c++ -DKCOREADDONS_LIB -DKF_DEPRECATED_WARNINGS_SINCE=0x060000 -DQT_CORE_LIB -DQT_DBUS_LIB -DQT_DEPRECATED_WARNINGS_SINCE=0x060000 -DQT_GUI_LIB -DQT_NO_CAST_FROM_ASCII -DQT_NO_CAST_FROM_BYTEARRAY -DQT_NO_CAST_TO_ASCII -DQT_NO_DEBUG -DQT_NO_NARROWING_CONVERSIONS_IN_CONNECT -DQT_NO_SIGNALS_SLOTS_KEYWORDS -DQT_NO_URL_CAST_FROM_STRING -DQT_STRICT_ITERATORS -DQT_USE_QSTRINGBUILDER -D_GNU_SOURCE -D_LARGEFILE64_SOURCE -Dkwayland_EXPORTS -Ibackends/wayland/plugins/kwayland -I/tmp/makepkg/disman-git/src/disman/backends/wayland/plugins/kwayland -Ibackends/wayland/plugins/kwayland/kwayland_autogen/include -Ibackends/wayland -I/tmp/makepkg/disman-git/src/disman/backends/wayland -I/tmp/makepkg/disman-git/src/disman/lib/lib -Ilib -I/tmp/makepkg/disman-git/src/disman/lib -isystem /usr/include/qt -isystem /usr/include/qt/QtCore -isystem /usr/lib/qt/mkspecs/linux-g++ -isystem /usr/include/qt/QtDBus -isystem /usr/include/qt/QtGui -isystem /usr/include/KF5/KCoreAddons -isystem /usr/include/KF5 -isystem /usr/include/KF5/KWayland/Client -D_FORTIFY_SOURCE=2 -march=native -O2 -pipe -fno-plt -ftree-vectorize -ftree-slp-vectorize -fno-operator-names -fno-exceptions -Wall -Wextra -Wcast-align -Wchar-subscripts -Wformat-security -Wno-long-long -Wpointer-arith -Wundef -Wnon-virtual-dtor -Woverloaded-virtual -Werror=return-type -Wvla -Wdate-time -Wsuggest-override -Wlogical-op -fdiagnostics-color=always -pedantic -Wzero-as-null-pointer-constant -O3 -DNDEBUG -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -fPIC -std=c++17 -MD -MT backends/wayland/plugins/kwayland/CMakeFiles/kwayland.dir/kwayland_output.cpp.o -MF backends/wayland/plugins/kwayland/CMakeFiles/kwayland.dir/kwayland_output.cpp.o.d -o backends/wayland/plugins/kwayland/CMakeFiles/kwayland.dir/kwayland_output.cpp.o -c /tmp/makepkg/disman-git/src/disman/backends/wayland/plugins/kwayland/kwayland_output.cpp
/tmp/makepkg/disman-git/src/disman/backends/wayland/plugins/kwayland/kwayland_output.cpp: In function ‘KWayland::Client::OutputDevice::Transform toKWaylandTransform(Disman::Output::Rotation)’:
/tmp/makepkg/disman-git/src/disman/backends/wayland/plugins/kwayland/kwayland_output.cpp:56:1: error: control reaches end of non-void function [-Werror=return-type]
   56 | }
      | ^
cc1plus: some warnings being treated as errors
ninja: build stopped: subcommand failed.
romangg commented 3 years ago

In GitLab by @Valmar on Sep 26, 2020, 07:26

I'm using GCC 10.2.0 on Arch Linux, by the way.

romangg commented 3 years ago

In GitLab by @Valmar on Sep 26, 2020, 07:37

Ah, I see... it compiles when I'm explicitly forcing -O2 ~ but it fails when CMAKE_BUILD_TYPE is set to Release...

romangg commented 3 years ago

Thanks for the feedback! I will try to fix this immediately. That looks like a simple issue with a removed assert in the release build and in this case no function return.

romangg commented 3 years ago

The assert that is removed on release build is here.

romangg commented 3 years ago

In GitLab by @Valmar on Sep 26, 2020, 13:08

Cheers. :)

It compiled when I set CMAKE_BUILD_TYPE to None and set CMAKE_C_FLAGS and CMAKE_CXX_FLAGS to use Arch's default makeflags, which include -O2.

For more explanation, I suspect that CMAKE_BUILD_TYPE being set to Release sets the makeflags to -O0. As, according to StackOverflow, this particular error is caused when -O0 is being used.

romangg commented 3 years ago

In GitLab by @haagch on Sep 26, 2020, 13:23

Asserts are disabled when cmake adds -DNDEBUG to the cflags and it does that in Release mode.

Sticking

string(REPLACE "-DNDEBUG" "" CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE}")

at the top of the top level CMakeLists.txt will enable asserts and allow to build in release mode but I'm sure it will be fixed soon anyway.

romangg commented 3 years ago

@Valmar I have opened !16 to fix the issue. I tested it locally but if you want to you can try out the branch in this MR to check if this fixes the release build also for you.

But since I tested the release build here you shouldn't have to. I would just push to master and then cherry-pick to the stable branch.

romangg commented 3 years ago

mentioned in commit 42bb9071a7b3390808d0a335f22b51e26ff48e07

romangg commented 3 years ago

The fix was cherry-picked to Plasma/5.20 branch by f6622f4d025fa9e8929b3bd143b12b590dbb0685.