oclero / qlementine

⚠️🏗️ [WORK IN PROGRESS] 🍊Modern QStyle for desktop Qt5/Qt6 applications.
https://oclero.github.io/qlementine/
MIT License
71 stars 14 forks source link

Build fixes for GCC 13 and MinGW compilers #52

Closed hhromic closed 4 months ago

hhromic commented 4 months ago

This PR fixes two build issues I found while preparing new updated Solarus build images that support Qlementine.

  1. Fix dangling reference warnings in GCC 13
  2. Use correct preprocessor macro for Windows code paths

Each of these is fixed on an individual commit for an easier review, but explanations are also provided below.

In GCC 13, a new dangling references warning was introduced. Qlementine has code that is triggering this warning and failing to build due to -Werror. This PR fixes the warnings-turned-errors by using intermediate assignments instead.

The first commit in this PR fixes these dangling reference warnings. ``` /qlementine/lib/src/widgets/Switch.cpp: In member function ‘const QColor& oclero::qlementine::Switch::getBgColor() const’: /qlementine/lib/src/widgets/Switch.cpp:251:15: error: possibly dangling reference to a temporary [-Werror=dangling-reference] 251 | const auto& bgColor = qlementineStyle ? qlementineStyle->switchGrooveColor( | ^~~~~~~ /qlementine/lib/src/widgets/Switch.cpp:253:73: note: the temporary was destroyed at the end of the full expression ‘QStyle::standardPalette().QPalette::color((((const oclero::qlementine::Switch*)this)->oclero::qlementine::Switch::.QAbstractButton::.QWidget::isEnabled() ? QPalette::Normal : QPalette::Disabled), QPalette::Button)’ 253 | : style->standardPalette().color( | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^ 254 | isEnabled() ? QPalette::ColorGroup::Normal : QPalette::ColorGroup::Disabled, | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 255 | QPalette::ColorRole::Button); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /qlementine/lib/src/widgets/Switch.cpp: In member function ‘const QColor& oclero::qlementine::Switch::getBorderColor() const’: /qlementine/lib/src/widgets/Switch.cpp:262:15: error: possibly dangling reference to a temporary [-Werror=dangling-reference] 262 | const auto& borderColor = | ^~~~~~~~~~~ /qlementine/lib/src/widgets/Switch.cpp:266:39: note: the temporary was destroyed at the end of the full expression ‘QStyle::standardPalette().QPalette::color((((const oclero::qlementine::Switch*)this)->oclero::qlementine::Switch::.QAbstractButton::.QWidget::isEnabled() ? QPalette::Normal : QPalette::Disabled), QPalette::ButtonText)’ 266 | : style->standardPalette().color( | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^ 267 | isEnabled() ? QPalette::ColorGroup::Normal : QPalette::ColorGroup::Disabled, QPalette::ColorRole::ButtonText); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /qlementine/lib/src/widgets/Switch.cpp: In member function ‘const QColor& oclero::qlementine::Switch::getFgColor() const’: /qlementine/lib/src/widgets/Switch.cpp:274:15: error: possibly dangling reference to a temporary [-Werror=dangling-reference] 274 | const auto& fgColor = qlementineStyle ? qlementineStyle->switchHandleColor( | ^~~~~~~ /qlementine/lib/src/widgets/Switch.cpp:276:73: note: the temporary was destroyed at the end of the full expression ‘QStyle::standardPalette().QPalette::color((((const oclero::qlementine::Switch*)this)->oclero::qlementine::Switch::.QAbstractButton::.QWidget::isEnabled() ? QPalette::Normal : QPalette::Disabled), QPalette::ButtonText)’ 276 | : style->standardPalette().color( | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^ 277 | isEnabled() ? QPalette::ColorGroup::Normal : QPalette::ColorGroup::Disabled, | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 278 | QPalette::ColorRole::ButtonText); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /qlementine/lib/src/widgets/Switch.cpp: In member function ‘const QColor& oclero::qlementine::Switch::getTextColor() const’: /qlementine/lib/src/widgets/Switch.cpp:285:15: error: possibly dangling reference to a temporary [-Werror=dangling-reference] 285 | const auto& textColor = | ^~~~~~~~~ /qlementine/lib/src/widgets/Switch.cpp:288:39: note: the temporary was destroyed at the end of the full expression ‘QStyle::standardPalette().QPalette::color((((const oclero::qlementine::Switch*)this)->oclero::qlementine::Switch::.QAbstractButton::.QWidget::isEnabled() ? QPalette::Normal : QPalette::Disabled), QPalette::Text)’ 288 | : style->standardPalette().color( | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^ 289 | isEnabled() ? QPalette::ColorGroup::Normal : QPalette::ColorGroup::Disabled, QPalette::ColorRole::Text); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ```

The second build error is due to (incorrectly) using a WIN32 user macro instead of the standard predefined _WIN32 compiler macro. When compiling with MSVC, it automatically adds the user defined macro WIN32 for backwards compatibility, but this is not the case for other compilers such as MinGW or Clang. Therefore, when compiling with these, the wrong code paths are chosen causing a missing symbol in the built library that then causes a linking error in client applications.

The second commit replaces `WIN32` with `_WIN32` and fixes this linking error in client applications. ``` [ 98%] Linking CXX executable sandbox.exe /usr/lib/gcc/x86_64-w64-mingw32/13.1.0/../../../../x86_64-w64-mingw32/bin/ld: ../lib/libqlementine.a(ResourceInitialization.cpp.obj):ResourceInitialization.cpp:(.text+0x18): undefined reference to `qInitResources_qlementine_font_inter()' /usr/lib/gcc/x86_64-w64-mingw32/13.1.0/../../../../x86_64-w64-mingw32/bin/ld: ../lib/libqlementine.a(ResourceInitialization.cpp.obj):ResourceInitialization.cpp:(.text+0x38): undefined reference to `qInitResources_qlementine_font_inter()' collect2: error: ld returned 1 exit status make[2]: *** [sandbox/CMakeFiles/sandbox.dir/build.make:185: sandbox/sandbox.exe] Error 1 make[1]: *** [CMakeFiles/Makefile2:203: sandbox/CMakeFiles/sandbox.dir/all] Error 2 make: *** [Makefile:91: all] Error 2 ```

EDIT: I tried to run clang-format but it formatted more code than my own changes in this PR, therefore I think is better to run it outside of this particular PR to reduce diff noise.