Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] (bug summary)\macros.h:418:20: warning: comparison between signed and unsigned integer #25459

Closed
1 task
rickjs opened this issue Mar 4, 2023 · 3 comments · Fixed by #25613
Closed
1 task

Comments

@rickjs
Copy link

rickjs commented Mar 4, 2023

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

No, but I will test it now!

Bug Description

In file included from e:\marlin2\marlin\src\inc\MarlinConfigPre.h:37:0,
from e:\marlin2\marlin\src\inc\marlinconfig.h:28,
from Marlin\src\gcode\feature\trinamic\M122.cpp:23:
e:\marlin2\marlin\src\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\gcode\feature\trinamic\M122.cpp:50:64: required from here
e:\marlin2\marlin\src\core\macros.h:418:20: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
return lhs > rhs ? lhs : rhs;

Bug Timeline

?

Expected behavior

Compile without this error

Actual behavior

No response

Steps to Reproduce

Compiling using Platformio

Version of Marlin Firmware

2.0.x bugfix(3/2/23)

Printer model

Prusa MK3S - Einsy

Electronics

No response

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

Chatgpt suggests...

This warning suggests that there is a comparison being made between a signed integer (lhs) and an unsigned integer (rhs). Since signed and unsigned integers are stored differently in memory, this can cause unexpected behavior and lead to bugs in your code.

To resolve this warning, you can modify the _MAX function in the macros.h file to use the same data type for both the lhs and rhs parameters. One possible solution is to add a type cast to convert the unsigned integer to a signed integer before making the comparison: c++

template<typename L, typename R>
constexpr decltype(auto) _MAX(L lhs, R rhs) {
return lhs > static_cast(rhs) ? lhs : static_cast(rhs);
}

By adding the static_cast(rhs) expression, we are explicitly converting the rhs parameter to the same data type as lhs, which will resolve the warning and ensure that the function works as intended.

However, it's important to note that modifying the macros.h file can have unintended consequences and cause other errors or issues. Therefore, it's recommended to test your changes thoroughly and seek assistance from the Marlin firmware community or developers if you're unsure about how to proceed.
Configuration.h.txt
Configuration_adv.h.txt

@sjasonsmith
Copy link
Contributor

I've confirmed the issue and posted a PR to fix it in bugfix-2.1.x.

@thisiskeithb thisiskeithb linked a pull request Apr 1, 2023 that will close this issue
@thisiskeithb
Copy link
Member

Closing since there’s now a PR to fix this:

@github-actions
Copy link

github-actions bot commented Jun 1, 2023

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.

@github-actions github-actions bot locked and limited conversation to collaborators Jun 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants