[BUG] Warning: comparison of integer expressions of different signedness: 'const int' and 'const unsigned int'

[classicrocker883]

Did you test the latest bugfix-2.1.x code?

Yes, and the problem still exists.

Bug Description

I noticed this recently:
Link to where the warning is present under "> Run STM32F103RE_creality Tests"

CrealityUI configs (to test on your own)

In file included from Marlin\src\module\../inc/MarlinConfigPre.h:37,
                 from Marlin\src\module\../inc/MarlinConfig.h:28,
                 from Marlin\src\module\stepper.h:44,
                 from Marlin\src\module\stepper.cpp:86:
Marlin\src\module\../inc/../core/macros.h: In instantiation of 'constexpr decltype ((lhs + rhs)) _MAX(L, R) [with L = int; R = unsigned int; decltype ((lhs + rhs)) = unsigned int]':
Marlin\src\module\stepper.h:302:13:   required from here
Marlin\src\module\../inc/../core/macros.h:395:20: warning: comparison of integer expressions of different signedness: 'const int' and 'const unsigned int' [-Wsign-compare]
  395 |         return lhs > rhs ? lhs : rhs;

The code in question:
Marlin\src\module\stepper.h:302 -
_MAX((STEPPER_TIMER_RATE) / HAL_TIMER_TYPE_MAX, 1U)

So, the warning goes away when removing the U in 1U so it is just 1

Also, another workaround is under macros.h:

  #ifndef _MINMAX_H_
  #define _MINMAX_H_

  #include <type_traits>

    extern "C++" {
...

      template <class L, class R> static constexpr auto _MAX(const L lhs, const R rhs) -> typename std::common_type<L, R>::type {
        using common_type = typename std::common_type<L, R>::type;
        return (static_cast<common_type>(lhs) > static_cast<common_type>(rhs)) ? static_cast<common_type>(lhs) : static_cast<common_type>(rhs);
      }

however, this is probably related to a recent commit, so that should be looked into.

Bug Timeline

Within the past couple days. You can look at a pull request check (STM32F103RE_creality) from 4 days ago (since 7/15) and the warning is not there

Expected behavior

Should be no warning

Actual behavior

Warning when using Creality configs/CI Build Tests (Actions)

Steps to Reproduce

1. Load up CrealityUI configs
2. STM32F103RE_creality in platformio.ini
3. Build - observe terminal warnings

Option B:

1. View a recent pull request (within a couple days of 7/15)
2. Under checks look for "CI - Build Tests / Build Test (STM32F103RE_creality) (pull_request)"
3. Click Details
4. Under "> Run STM32F103RE_creality Tests" you should see the multiple (same) warnings

Version of Marlin Firmware

bugfix-2.1.x

Printer model

Creality Ender-3

Electronics

BOARD_CREALITY_V4

LCD/Controller

DWIN_CREALITY_LCD

Other add-ons

No response

Bed Leveling

None

Your Slicer

None

Host Software

None

Don't forget to include

• A ZIP file containing your Configuration.h and Configuration_adv.h.

Additional information & file uploads

Configs used

[ellensp]

#26881 from 2 days ago.
This was added to stepper.h
_MAX((STEPPER_TIMER_RATE) / HAL_TIMER_TYPE_MAX, 1U) // 32-bit shouldn't go below 1

This triggers the warning.

[ellensp]

I purpose

diff --git a/Marlin/src/module/stepper.h b/Marlin/src/module/stepper.h
index 2a171bebd0..c24953c24e 100644
--- a/Marlin/src/module/stepper.h
+++ b/Marlin/src/module/stepper.h
@@ -299,7 +299,7 @@ class Stepper {
     // and avoid the most unreasonably slow step rates.
     static constexpr uint32_t minimal_step_rate = (
       #ifdef CPU_32_BIT
-        _MAX((STEPPER_TIMER_RATE) / HAL_TIMER_TYPE_MAX, 1U) // 32-bit shouldn't go below 1
+        _MAX((uint32_t(STEPPER_TIMER_RATE) / HAL_TIMER_TYPE_MAX), 1U) // 32-bit shouldn't go below 1
       #else
         (F_CPU) / 500000U   // AVR shouldn't go below 32 (16MHz) or 40 (20MHz)
       #endif

as STEPPER_TIMER_RATE/HAL_TIMER_TYPE_MAX can never be negative

[classicrocker883]

why not just make it _MAX((STEPPER_TIMER_RATE) / HAL_TIMER_TYPE_MAX, 1)?

the end result would be no different.

[ellensp]

Int comparisons are faster than signed and can use less code, does it make a lot of different, no, just marginally better

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.